fix(config): replace sync.Once reset-from-Do with mutex

Load()'s validation-failure path reassigned cfgOnce = sync.Once{} from
inside Do(). When Do() returned and tried to unlock the original mutex,
the Once struct had already been replaced with a fresh one whose mutex
was unlocked, panicking with "sync: unlock of unlocked mutex" on every
boot where any required env var was missing or invalid.

Replaced the Once with a plain sync.Mutex around a nil-check on the
package-level cfg, building the candidate into a local first and only
assigning to cfg after validate() succeeds. Same caching semantics, no
race, and a failed Load() leaves cfg nil so the next caller retries
cleanly.

Also documented AppleAuthConfig.TeamID as currently dead — it's loaded
from APPLE_TEAM_ID but no service reads it. Wire-up point noted for
when Sign in with Apple revocation/refresh is added.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Trey t
2026-05-01 08:35:54 -07:00
parent 289a23f7e6
commit 8fce568532
2 changed files with 164 additions and 160 deletions
+23 -19
View File
@@ -89,8 +89,12 @@ type PushConfig struct {
}
type AppleAuthConfig struct {
ClientID string // Bundle ID (e.g., com.tt.honeyDue.honeyDueDev)
TeamID string // Apple Developer Team ID
ClientID string // Bundle ID, used as the `aud` claim in Sign in with Apple identity tokens
// TeamID is currently unused — services/apple_auth.go validates identity tokens
// against ClientID + Apple's JWKS only, with no server-to-server REST calls.
// Wire this in if/when token revocation or refresh-token exchange is added,
// since both require signing a client_secret JWT with team_id + key_id.
TeamID string
}
type GoogleAuthConfig struct {
@@ -179,7 +183,7 @@ type FeatureFlags struct {
var (
cfg *Config
cfgOnce sync.Once
cfgMu sync.Mutex
)
// knownWeakSecretKeys contains well-known default or placeholder secret keys
@@ -192,11 +196,19 @@ var knownWeakSecretKeys = map[string]bool{
"change-me-in-production-secret-key-12345": true,
}
// Load reads configuration from environment variables
// Load reads configuration from environment variables.
//
// Caches the result so repeated calls are cheap. On validation failure, the
// cache stays nil so a subsequent call (after env is corrected) can retry. The
// previous implementation used sync.Once with an in-Do reset of the Once
// itself, which races and panics with "sync: unlock of unlocked mutex".
func Load() (*Config, error) {
var loadErr error
cfgMu.Lock()
defer cfgMu.Unlock()
if cfg != nil {
return cfg, nil
}
cfgOnce.Do(func() {
viper.SetEnvPrefix("")
viper.AutomaticEnv()
viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
@@ -236,7 +248,7 @@ func Load() (*Config, error) {
}
}
cfg = &Config{
c := &Config{
Server: ServerConfig{
Port: viper.GetInt("PORT"),
Debug: viper.GetBool("DEBUG"),
@@ -336,19 +348,11 @@ func Load() (*Config, error) {
},
}
// Validate required fields
if err := validate(cfg); err != nil {
loadErr = err
// Reset so a subsequent call can retry after env is fixed
cfg = nil
cfgOnce = sync.Once{}
if err := validate(c); err != nil {
// Leave cfg nil so the next Load() retries after env is corrected.
return nil, err
}
})
if loadErr != nil {
return nil, loadErr
}
cfg = c
return cfg, nil
}
+2 -2
View File
@@ -1,7 +1,6 @@
package config
import (
"sync"
"testing"
"github.com/spf13/viper"
@@ -11,8 +10,9 @@ import (
// resetConfigState resets the package-level singleton so each test starts fresh.
func resetConfigState() {
cfgMu.Lock()
cfg = nil
cfgOnce = sync.Once{}
cfgMu.Unlock()
viper.Reset()
}