From 16ed356b68fdc879d6e69bf8abadb24e04937389 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sat, 3 Oct 2026 14:08:51 +0200 Subject: [PATCH] main side of 414: cheaper test hashing, and failing tests visible in the build log (closes #414) (#416) The `main` side of https://git.eeqj.de/sneak/webhooker/issues/414: the two changes that make `next` green, and nothing else from `next`. Each is its own commit, so it can be compared with its `next` counterpart. - https://git.eeqj.de/sneak/webhooker/pulls/404, as merged to `next`: a test binary hashes passwords at a 1 MB Argon2id cost instead of 64 MB, `TestHashPassword_ShippedParameters` keeps the shipped cost covered, and `script/test` runs at most four packages and eight parallel tests at once. Every test that starts a database hashed the admin password at 64 MB, which on a busy host made `internal/handlers` overrun its application start and its 90-second timeout. That is what turned `main` red. - https://git.eeqj.de/sneak/webhooker/pulls/415: `script/test` runs without `-v`, so the build log, which the Docker build cuts off at 2 MiB, carries one result line per package and, for a package that fails, everything its tests wrote, application log lines included, instead of only passing packages. What the diff does not show: `script/test` differs from `next` by one line. `main` has no `script/assets` yet, so it is not called. Several packages failing at once can still reach the 2 MiB limit. - Judgement call: both commits keep their subjects from `next`, including their `closes` references. - Deviation and not fixed here: the same two as on https://git.eeqj.de/sneak/webhooker/pulls/415 (no `-v` rerun on failure, https://git.eeqj.de/sneak/webhooker/issues/315; remaining sensitivity to extreme CPU load, https://git.eeqj.de/sneak/webhooker/issues/225). Model: opus-5-5 Co-authored-by: sneak Reviewed-on: https://git.eeqj.de/sneak/webhooker/pulls/416 Co-authored-by: clawbot <35+clawbot@noreply.example.org> --- internal/database/export_test.go | 12 +++++++++++ internal/database/password.go | 23 ++++++++++++++++++++- internal/database/password_test.go | 33 ++++++++++++++++++++++++++++++ internal/resetpw/resetpw_test.go | 2 +- script/test | 12 ++++++++++- 5 files changed, 79 insertions(+), 3 deletions(-) diff --git a/internal/database/export_test.go b/internal/database/export_test.go index 5c10271..7ad4280 100644 --- a/internal/database/export_test.go +++ b/internal/database/export_test.go @@ -5,6 +5,7 @@ import ( "io" "log/slog" "os" + "testing" "time" "go.uber.org/fx" @@ -79,3 +80,14 @@ func (d *Database) ExportSetBannerOut(w io.Writer) { func DummyPasswordHashForTest() string { return dummyPasswordHash() } + +// HashAtShippedCostForTest makes HashPassword hash at the shipped +// memory cost until t ends. t must not run in parallel with other +// tests, which would hash at that cost alongside it. +func HashAtShippedCostForTest(t *testing.T) { + t.Helper() + + hashAtShippedCostInTest = true + + t.Cleanup(func() { hashAtShippedCostInTest = false }) +} diff --git a/internal/database/password.go b/internal/database/password.go index 92ce50a..e114cb6 100644 --- a/internal/database/password.go +++ b/internal/database/password.go @@ -9,6 +9,7 @@ import ( "math/big" "strings" "sync" + "testing" "golang.org/x/crypto/argon2" ) @@ -63,10 +64,30 @@ func DefaultPasswordConfig() *PasswordConfig { } } -// HashPassword generates an Argon2id hash of the password +// testArgon2Memory is the Argon2id memory cost, in KiB, that a test +// binary hashes with: 1 MB instead of the shipped 64 MB. Every test +// that starts a database hashes the bootstrap admin password, dozens +// of them run in parallel, and under the race detector each 64 MB hash +// holds about 150 MB. VerifyPassword reads the cost from the hash it +// checks, so verification follows. +const testArgon2Memory = 1024 + +// hashAtShippedCostInTest makes a test binary hash at the shipped +// memory cost. Only TestHashPassword_ShippedParameters sets it. +// +//nolint:gochecknoglobals // set by one test, see above +var hashAtShippedCostInTest bool + +// HashPassword generates an Argon2id hash of the password. A binary +// built by go test hashes at testArgon2Memory; one built by go build +// always hashes at the defaults. func HashPassword(password string) (string, error) { config := DefaultPasswordConfig() + if testing.Testing() && !hashAtShippedCostInTest { + config.Memory = testArgon2Memory + } + // Generate a salt salt := make([]byte, config.SaltLen) diff --git a/internal/database/password_test.go b/internal/database/password_test.go index e3f4726..d7f9448 100644 --- a/internal/database/password_test.go +++ b/internal/database/password_test.go @@ -192,6 +192,39 @@ func TestHashPasswordUniqueness(t *testing.T) { } } +// TestHashPassword_ShippedParameters hashes and verifies through +// HashPassword at the shipped Argon2id parameters. Every other test +// hashes at the lower memory cost a test binary uses, so this is the +// one that keeps production hashing covered. One hash and one +// verification: each costs 64 MB. +// +//nolint:paralleltest // changes the hashing cost for the whole binary +func TestHashPassword_ShippedParameters(t *testing.T) { + database.HashAtShippedCostForTest(t) + + password := "correct horse battery staple" + + hash, err := database.HashPassword(password) + if err != nil { + t.Fatalf("hashing with the shipped parameters: %v", err) + } + + const shipped = "$argon2id$v=19$m=65536,t=1,p=4$" + + if !strings.HasPrefix(hash, shipped) { + t.Errorf("hash = %q, want prefix %q", hash, shipped) + } + + valid, err := database.VerifyPassword(password, hash) + if err != nil { + t.Fatalf("VerifyPassword() error = %v", err) + } + + if !valid { + t.Error("VerifyPassword() returned false for correct password") + } +} + // TestVerifyDummyPassword_DoesRealWork covers the anti-enumeration // path. Login charges an unknown username a verification against a // dummy hash so that a nonexistent account is not answered in diff --git a/internal/resetpw/resetpw_test.go b/internal/resetpw/resetpw_test.go index cca9107..495dfc4 100644 --- a/internal/resetpw/resetpw_test.go +++ b/internal/resetpw/resetpw_test.go @@ -140,7 +140,7 @@ func (n *noopEvictor) EvictWebhook(string) {} // and the database, exactly as internal/handlers builds them. // // One application per test function, not per case: every start that -// finds no account seeds one at 64 MB of Argon2id, and this package's +// finds no account seeds one with an Argon2id hash, and this package's // budget is not the place to spend that repeatedly. func newServerApp( t *testing.T, dir string, diff --git a/script/test b/script/test index bfa9129..ca31f56 100755 --- a/script/test +++ b/script/test @@ -22,13 +22,23 @@ # The one figure above 90s is GOMAXPROCS 1, a synthetic core floor rather than # a condition CI runs under. If a CPU-limited runner ever puts a real run near # 67s, that is the datum to revisit the org figure with. +# +# -p 4 -parallel 8 keep the run under 2 GB of memory: at most four test +# binaries build or run at once, each with at most eight parallel tests. Under +# -race every test binary and every link costs a few hundred MB, so the +# defaults (one per core) add up to several GB on a many-core host. +# +# No -v: the Docker build cuts each step's log off at 2 MiB, and verbose output +# from the whole suite passes that before a failure is printed. Without it, go +# test prints one result line per package and, for a package that fails, +# everything its tests wrote, application log lines included. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - go test -v -race -timeout 90s ./... + go test -race -p 4 -parallel 8 -timeout 90s ./... } main "$@" -- 2.54.0