perf(users/test): make bcrypt cost cheap under go test#61
Merged
Conversation
Password hashing uses bcrypt cost 12 (~250ms/hash by design). Because nearly every server test seeds a user through a fixture, that deliberate cost dominated the whole `-race` suite: httpapi ~462s, users ~83s, sessions ~43s, apikeys ~28s — almost entirely bcrypt, serialized and amplified by CI CPU contention. Resolve the work factor once at init instead of a hard-coded const: 1. CIX_BCRYPT_COST — explicit override, clamped to [MinCost, MaxCost] 2. testing.Testing() — drop to bcrypt.MinCost under `go test` 3. 12 — production default (unchanged behaviour outside tests) testing.Testing() is false outside test binaries, and since Go 1.13 importing "testing" no longer registers test flags, so cix-server's flag.Parse() is unaffected (verified: `cix-server -v` still works). Also derive the user-not-found anti-enumeration dummy hash from the active cost (was a hard-coded `$2a$12$…` literal) so the timing mitigation stays accurate when the cost changes and the not-found path is cheap under test too. Measured (local, `go test -race -count=1`): httpapi 462.3s → 15.8s users 83.0s → <1s sessions 43.3s → <1s apikeys 27.9s → 5.4s full suite all green, 0 data races, ~23s wall. chunker (~19s, tree-sitter/CGO) is the remaining slow package and is inherent to -race; left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Password hashing uses bcrypt cost 12 (~250 ms/hash, by design). Almost every
server test seeds a user through a fixture, so that deliberate cost dominated the
whole
-racesuite. This resolves the work factor once at init and drops it tobcrypt.MinCostundergo test.Why
Per-package
go test -race ./...times were mostly pure bcrypt, serialized andamplified by CI CPU contention:
Full suite after the fix: all green, 0 data races, ~23 s wall locally.
(
chunkeris tree-sitter/CGO — inherently slow under-race, left as-is.)How
internal/users/users.go—BcryptCostgoes from aconstto a value resolvedonce at package init, highest precedence first:
CIX_BCRYPT_COST— explicit override, clamped to[bcrypt.MinCost, MaxCost].testing.Testing()— undergo test, usebcrypt.MinCost(~256× cheaper).The hash is never what the tests assert; round-trip behaviour is identical.
12— production default (unchanged outside tests).Safety:
testing.Testing()is false outside test binaries, and since Go 1.13importing
testingno longer registers test flags, socix-server'sflag.Parse()is unaffected — verifiedcix-server -vstill works. We're onGo 1.25.
Also derived the user-not-found anti-enumeration dummy hash from the active cost
(was a hard-coded
$2a$12$…literal) so the timing mitigation tracks the costand the not-found path is cheap under test too.
Type of change
Checklist
go test -race ./...passes — all green, 0 data racesgo vet ./internal/users/passescix-serverbuilds and-vruns (testing import doesn't break flag parsing)🤖 Generated with Claude Code