From 5eeb37389bfb440ab7d787d8b35d33cf6a916c96 Mon Sep 17 00:00:00 2001 From: Huxley Date: Sun, 5 Jul 2026 13:25:44 +0800 Subject: [PATCH] fix(config): harden default JWT secret Replace known development JWT secret placeholders with an ephemeral runtime secret during config loading. Return LoadInfo so startup can warn when token persistence depends on a stable configured secret. Update config docs and tests for default, legacy placeholder, custom, env, and empty secret behavior. --- cmd/serve.go | 7 +++- config.example.yaml | 4 ++- docs/decisions.md | 2 ++ docs/development.md | 7 +++- internal/config/load.go | 61 +++++++++++++++++++++++++++++---- internal/config/load_test.go | 66 ++++++++++++++++++++++++++++++++---- 6 files changed, 131 insertions(+), 16 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index ae94f44..2426de5 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -23,7 +23,7 @@ var serveCmd = &cobra.Command{ Short: "Start the MyGO HTTP server", RunE: func(cmd *cobra.Command, args []string) error { v := config.New() - cfg, err := config.Load(v, serveConfigFile) + cfg, loadInfo, err := config.Load(v, serveConfigFile) if err != nil { return fmt.Errorf("load config: %w", err) } @@ -32,6 +32,11 @@ var serveCmd = &cobra.Command{ appLogger := mygolog.NewLogger(cfg.Log) slog.SetDefault(appLogger) slog.Info("mygo server starting") + if loadInfo.EphemeralJWTSecret { + slog.Warn("jwt.secret is a development placeholder; using an ephemeral runtime secret. " + + "Set a stable jwt.secret or MYGO_JWT_SECRET for production, multi-instance deployments, " + + "and token persistence across restarts") + } ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() diff --git a/config.example.yaml b/config.example.yaml index 47340f1..d9a652b 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -28,6 +28,8 @@ log: file_level: debug # file: debug, info, warn, error jwt: - secret: change-me-in-production + # Development placeholder. MyGO replaces this with an ephemeral runtime + # secret at startup; set a stable value for production. + secret: dev-secret-do-not-use-in-production access_ttl: 15m refresh_ttl: 168h diff --git a/docs/decisions.md b/docs/decisions.md index 97574e2..2dd4732 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -59,9 +59,11 @@ |----------|----------| | One handler per route group | `AuthHandler` owns `/auth/*` (public); `AccountHandler` owns `/account/*` (protected). A route group maps 1:1 to a handler type. | | JWT `type` claim | `Claims.Type` distinguishes access from refresh tokens. Middleware and service enforce the correct type at their respective boundaries. `ParseToken` does no type check — it verifies cryptographic validity only. | +| Default JWT secret hardening | The development placeholder `jwt.secret` is replaced with an ephemeral runtime secret during config loading. Production and multi-instance deployments must set a stable secret. | | `time.Duration` in config structs | Config fields representing durations use `time.Duration` directly. Viper's built-in `StringToTimeDurationHookFunc` handles string→Duration conversion at unmarshal time. No accessor methods, no runtime parsing. Invalid values fail at startup via `Load()`. | **Consequences**: - Handlers are independently extensible (caching, rate limiting scoped per handler). - Refresh tokens cannot authenticate API requests; access tokens cannot be used to issue new token pairs. +- The placeholder JWT secret is safe for local startup, but tokens signed with it are invalidated on restart. - New duration config fields require zero boilerplate — declare as `time.Duration` in the struct. diff --git a/docs/development.md b/docs/development.md index a3004cf..644ae61 100644 --- a/docs/development.md +++ b/docs/development.md @@ -52,9 +52,14 @@ storage: path: data/files jwt: - secret: changeme-in-production + secret: dev-secret-do-not-use-in-production access_ttl: 15m refresh_ttl: 168h ``` Environment variables use `MYGO_` prefix with underscore separators: `MYGO_SERVER_PORT=8080`, `MYGO_JWT_SECRET=...` + +The default JWT secret is a development placeholder. At startup, MyGO replaces +it with an ephemeral runtime secret; set a stable `jwt.secret` or +`MYGO_JWT_SECRET` when tokens must survive restarts or when running multiple +instances. diff --git a/internal/config/load.go b/internal/config/load.go index 801382f..c343acf 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -1,6 +1,8 @@ package config import ( + "crypto/rand" + "encoding/base64" "errors" "fmt" "strings" @@ -8,6 +10,17 @@ import ( "github.com/spf13/viper" ) +// DefaultJWTSecret is a development-only placeholder. Load replaces it with +// an ephemeral secret before returning a usable Config. +const DefaultJWTSecret = "dev-secret-do-not-use-in-production" + +// LoadInfo describes runtime decisions made while loading configuration. +type LoadInfo struct { + // EphemeralJWTSecret is true when a placeholder jwt.secret was replaced + // with a random in-memory value for the current process. + EphemeralJWTSecret bool +} + func defaults(v *viper.Viper) { v.SetDefault("server.host", "0.0.0.0") v.SetDefault("server.port", 10086) @@ -24,7 +37,7 @@ func defaults(v *viper.Viper) { v.SetDefault("storage.driver", "local") v.SetDefault("storage.local.path", "data/files") - v.SetDefault("jwt.secret", "dev-secret-do-not-use-in-production") + v.SetDefault("jwt.secret", DefaultJWTSecret) v.SetDefault("jwt.access_ttl", "15m") v.SetDefault("jwt.refresh_ttl", "168h") @@ -42,7 +55,7 @@ func New() *viper.Viper { return v } -func Load(v *viper.Viper, cfgFile string) (*Config, error) { +func Load(v *viper.Viper, cfgFile string) (*Config, LoadInfo, error) { if cfgFile != "" { v.SetConfigFile(cfgFile) } else { @@ -54,18 +67,54 @@ func Load(v *viper.Viper, cfgFile string) (*Config, error) { if err := v.ReadInConfig(); err != nil { var notFound viper.ConfigFileNotFoundError if !errors.As(err, ¬Found) { - return nil, fmt.Errorf("read config: %w", err) + return nil, LoadInfo{}, fmt.Errorf("read config: %w", err) } } var cfg Config if err := v.Unmarshal(&cfg); err != nil { - return nil, fmt.Errorf("unmarshal config: %w", err) + return nil, LoadInfo{}, fmt.Errorf("unmarshal config: %w", err) + } + + info, err := hardenRuntimeSecrets(&cfg) + if err != nil { + return nil, LoadInfo{}, err } if err := cfg.Validate(); err != nil { - return nil, fmt.Errorf("validate config: %w", err) + return nil, LoadInfo{}, fmt.Errorf("validate config: %w", err) } - return &cfg, nil + return &cfg, info, nil +} + +func hardenRuntimeSecrets(cfg *Config) (LoadInfo, error) { + if !isWeakJWTSecret(cfg.JWT.Secret) { + return LoadInfo{}, nil + } + + secret, err := generateEphemeralJWTSecret() + if err != nil { + return LoadInfo{}, fmt.Errorf("generate ephemeral jwt secret: %w", err) + } + + cfg.JWT.Secret = secret + return LoadInfo{EphemeralJWTSecret: true}, nil +} + +func isWeakJWTSecret(secret string) bool { + switch secret { + case DefaultJWTSecret, "change-me-in-production", "changeme-in-production": + return true + default: + return false + } +} + +func generateEphemeralJWTSecret() (string, error) { + var secret [32]byte + if _, err := rand.Read(secret[:]); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(secret[:]), nil } diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 24e75a6..43d632b 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -9,7 +9,7 @@ import ( func TestDefaults(t *testing.T) { v := New() - cfg, err := Load(v, "") + cfg, info, err := Load(v, "") if err != nil { t.Fatal(err) } @@ -36,6 +36,16 @@ func TestDefaults(t *testing.T) { } }) } + + if !info.EphemeralJWTSecret { + t.Fatal("expected default jwt.secret to be replaced with an ephemeral secret") + } + if cfg.JWT.Secret == DefaultJWTSecret { + t.Fatal("default jwt.secret was not replaced") + } + if cfg.JWT.Secret == "" { + t.Fatal("ephemeral jwt.secret is empty") + } } func TestFromYAML(t *testing.T) { @@ -67,10 +77,13 @@ jwt: } v := New() - cfg, err := Load(v, path) + cfg, info, err := Load(v, path) if err != nil { t.Fatal(err) } + if info.EphemeralJWTSecret { + t.Fatal("custom jwt.secret should not be replaced") + } if cfg.Server.Host != "127.0.0.1" { t.Errorf("server.host = %q, want %q", cfg.Server.Host, "127.0.0.1") @@ -102,10 +115,13 @@ func TestEnvOverride(t *testing.T) { t.Setenv("MYGO_DATABASE_SQLITE_PATH", "/env/path/db.sqlite") v := New() - cfg, err := Load(v, "") + cfg, info, err := Load(v, "") if err != nil { t.Fatal(err) } + if info.EphemeralJWTSecret { + t.Fatal("env jwt.secret should not be replaced") + } if cfg.Server.Port != 8080 { t.Errorf("server.port = %d, want %d", cfg.Server.Port, 8080) @@ -137,7 +153,7 @@ server: t.Setenv("MYGO_SERVER_PORT", "9999") v := New() - cfg, err := Load(v, path) + cfg, _, err := Load(v, path) if err != nil { t.Fatal(err) } @@ -164,7 +180,7 @@ server: } v := New() - _, err := Load(v, path) + _, _, err := Load(v, path) if err == nil { t.Fatal("expected error for port 0, got nil") } @@ -183,7 +199,7 @@ jwt: } v := New() - _, err := Load(v, path) + _, _, err := Load(v, path) if err == nil { t.Fatal("expected error for empty jwt.secret, got nil") } @@ -191,12 +207,48 @@ jwt: func TestExplicitConfigFileNotFound(t *testing.T) { v := New() - _, err := Load(v, "/nonexistent/path/config.yaml") + _, _, err := Load(v, "/nonexistent/path/config.yaml") if err == nil { t.Fatal("expected error when explicitly specifying a nonexistent config file") } } +func TestWeakJWTSecretsAreReplaced(t *testing.T) { + tests := []string{ + DefaultJWTSecret, + "change-me-in-production", + "changeme-in-production", + } + + for _, secret := range tests { + t.Run(secret, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + + yaml := "jwt:\n secret: " + secret + "\n" + if err := os.WriteFile(path, []byte(yaml), 0644); err != nil { + t.Fatal(err) + } + + v := New() + cfg, info, err := Load(v, path) + if err != nil { + t.Fatal(err) + } + + if !info.EphemeralJWTSecret { + t.Fatal("expected weak jwt.secret to be replaced") + } + if cfg.JWT.Secret == secret { + t.Fatal("weak jwt.secret was not replaced") + } + if cfg.JWT.Secret == "" { + t.Fatal("ephemeral jwt.secret is empty") + } + }) + } +} + func TestJWTConfigAccessDuration(t *testing.T) { j := JWTConfig{AccessTTL: 15 * time.Minute} if j.AccessTTL != 15*time.Minute {