From cc29886fe0a1dcace95669209bd5db4c36de80e8 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:08:26 -0600 Subject: [PATCH 1/3] fix(config): use underscores for config keys/env vars, hyphens for flags A conflict in Viper naming schemas was causing a bug where a config file using underscore-separated config keys (which is correct according to the documentation) would get overridden by the default values, which use hyphen-separated keys. This could be fixed by using hyphen-separated keys in the config file, but this is neither idiomatic for YAML keys nor consistent with the config documentation. This is due to ambiguity in the way Viper config key aliases are used. The config struct uses underscores for mapstructure tags and flags use hyphens. Flags are aliased to config keys via viper.RegisterAlias("config_key", "config-key"), but this causes the unintentional side effect of hyphen-separated values being able to override underscore-separated values in an ambiguous way, such as the default values not being overridden by config file values. The intended behavior is strictly that: - CLI flags only use hyphen separation - config file and environment variable keys use underscore separation Therefore, this change removes the viper alias registration and adds a bindFlagsWithUnderscoreKeys() helper that dynamically binds hyphen-separated flags to underscore-separated config values. This ensures the rules stated above. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- cmd/server/main.go | 33 +++++++++------- cmd/server/main_test.go | 84 +++++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- 3 files changed, 104 insertions(+), 15 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index f7a6119..2951195 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -20,6 +20,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/openchami/boot-service/internal/storage" @@ -100,6 +101,21 @@ var serveCmd = &cobra.Command{ RunE: runServe, } +func bindFlagsWithUnderscoreKeys(v *viper.Viper, flags *pflag.FlagSet) error { + var bindErr error + + flags.VisitAll(func(flag *pflag.Flag) { + if bindErr != nil { + return + } + + key := strings.ReplaceAll(flag.Name, "-", "_") + bindErr = v.BindPFlag(key, flag) + }) + + return bindErr +} + func init() { // Server configuration flags serveCmd.Flags().Int("port", 8080, "Port to listen on") @@ -133,7 +149,9 @@ func init() { serveCmd.Flags().Int("hsm-sync-interval", 5, "HSM sync interval in minutes") // Bind flags to viper - viper.BindPFlags(serveCmd.Flags()) //nolint:errcheck + if err := bindFlagsWithUnderscoreKeys(viper.GetViper(), serveCmd.Flags()); err != nil { + panic(fmt.Errorf("failed to bind serve flags: %w", err)) + } // Add commands rootCmd.AddCommand(serveCmd) @@ -152,19 +170,6 @@ func main() { viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) viper.AutomaticEnv() - // Register aliases for flags with dashes to work with mapstructure tags that use underscores - viper.RegisterAlias("hsm_url", "hsm-url") - viper.RegisterAlias("hsm_sync_enabled", "hsm-sync-enabled") - viper.RegisterAlias("hsm_sync_interval", "hsm-sync-interval") - viper.RegisterAlias("tokensmith_bootstrap_token", "tokensmith-bootstrap-token") - viper.RegisterAlias("tokensmith_target_service", "tokensmith-target-service") - viper.RegisterAlias("tokensmith_bootstrap_policy_scopes_hint", "tokensmith-bootstrap-policy-scopes-hint") - viper.RegisterAlias("tokensmith_scopes", "tokensmith-scopes") - viper.RegisterAlias("tokensmith_refresh_skew_sec", "tokensmith-refresh-skew-sec") - viper.RegisterAlias("enable_auth", "enable-auth") - viper.RegisterAlias("enable_legacy_api", "enable-legacy-api") - viper.RegisterAlias("enable_metrics", "enable-metrics") - // Standardized TokenSmith env vars for cross-service UX consistency. viper.BindEnv("tokensmith_url", "TOKENSMITH_URL") //nolint:errcheck viper.BindEnv("tokensmith_bootstrap_token", "TOKENSMITH_BOOTSTRAP_TOKEN") //nolint:errcheck diff --git a/cmd/server/main_test.go b/cmd/server/main_test.go index bba0816..f6fbefa 100644 --- a/cmd/server/main_test.go +++ b/cmd/server/main_test.go @@ -17,12 +17,96 @@ import ( "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" + "github.com/spf13/pflag" + "github.com/spf13/viper" "github.com/openchami/boot-service/internal/storage" bootclient "github.com/openchami/boot-service/pkg/client" "github.com/openchami/boot-service/pkg/handlers/boot" ) +func TestBindFlagsWithUnderscoreKeys_ConfigValuesBeatUnchangedFlagDefaults(t *testing.T) { + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.Bool("enable-auth", false, "Enable authentication with TokenSmith") + flags.Bool("enable-metrics", false, "Enable Prometheus metrics") + flags.Bool("enable-legacy-api", true, "Enable legacy BSS API compatibility") + flags.String("tokensmith-url", "", "TokenSmith service URL for authentication") + flags.String("tokensmith-target-service", "hsm", "Target service audience for HSM service token exchange") + flags.String("hsm-url", "", "Hardware State Manager service URL") + flags.Bool("hsm-sync-enabled", true, "Enable background sync with HSM") + + v := viper.New() + if err := bindFlagsWithUnderscoreKeys(v, flags); err != nil { + t.Fatalf("bindFlagsWithUnderscoreKeys failed: %v", err) + } + + v.SetConfigType("yaml") + configYAML := ` +enable_auth: true +tokensmith_url: http://tokensmith:8080 +tokensmith_target_service: smd +hsm_url: http://smd:27779 +hsm_sync_enabled: true +enable_legacy_api: true +enable_metrics: false +` + if err := v.ReadConfig(strings.NewReader(configYAML)); err != nil { + t.Fatalf("ReadConfig failed: %v", err) + } + + config := DefaultConfig() + if err := v.Unmarshal(&config); err != nil { + t.Fatalf("Unmarshal failed: %v", err) + } + + if !config.EnableAuth { + t.Fatal("expected enable_auth config value to override unchanged --enable-auth default") + } + if config.TokenSmithURL != "http://tokensmith:8080" { + t.Fatalf("expected tokensmith_url from config, got %q", config.TokenSmithURL) + } + if config.TokenSmithTargetService != "smd" { + t.Fatalf("expected tokensmith_target_service from config, got %q", config.TokenSmithTargetService) + } + if config.HSMURL != "http://smd:27779" { + t.Fatalf("expected hsm_url config value to override unchanged --hsm-url default, got %q", config.HSMURL) + } + if !config.HSMSyncEnabled { + t.Fatal("expected hsm_sync_enabled from config") + } + if !config.EnableLegacyAPI { + t.Fatal("expected enable_legacy_api from config") + } + if config.EnableMetrics { + t.Fatal("expected enable_metrics to remain false") + } +} + +func TestBindFlagsWithUnderscoreKeys_ChangedHyphenatedFlagsUseUnderscoreKeys(t *testing.T) { + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.Bool("enable-auth", false, "Enable authentication with TokenSmith") + flags.String("hsm-url", "", "Hardware State Manager service URL") + + if err := flags.Set("enable-auth", "true"); err != nil { + t.Fatalf("Set enable-auth failed: %v", err) + } + if err := flags.Set("hsm-url", "http://smd:27779"); err != nil { + t.Fatalf("Set hsm-url failed: %v", err) + } + + v := viper.New() + if err := bindFlagsWithUnderscoreKeys(v, flags); err != nil { + t.Fatalf("bindFlagsWithUnderscoreKeys failed: %v", err) + } + + if !v.GetBool("enable_auth") { + t.Fatal("expected --enable-auth to bind to enable_auth") + } + if got := v.GetString("hsm_url"); got != "http://smd:27779" { + t.Fatalf("expected --hsm-url to bind to hsm_url, got %q", got) + } +} + func newGeneratedRouterForTest(t *testing.T) http.Handler { t.Helper() diff --git a/go.mod b/go.mod index a5f5dcd..3ce7a69 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/prometheus/client_golang v1.23.2 github.com/rs/zerolog v1.34.0 github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.11.1 gopkg.in/yaml.v3 v3.0.1 @@ -65,7 +66,6 @@ require ( github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect - github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/woodsbury/decimal128 v1.3.0 // indirect go.uber.org/multierr v1.11.0 // indirect From 581ed477a1c749ace430beead958101f2e00e8cd Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:38:50 -0600 Subject: [PATCH 2/3] fix(config): disable enable_legacy_api by default Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- cmd/server/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 2951195..4e66d5d 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -73,7 +73,7 @@ func DefaultConfig() Config { StorageType: "file", EnableAuth: false, EnableMetrics: false, - EnableLegacyAPI: true, + EnableLegacyAPI: false, MetricsPort: 9090, TokenSmithURL: "", TokenSmithBootstrapToken: "", From 1e38b2b196339b164a7b9b967306ec197c8675f7 Mon Sep 17 00:00:00 2001 From: Devon Bautista <17506592+synackd@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:39:08 -0600 Subject: [PATCH 3/3] fix: --tokensmith_url -> --tokensmith-url Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com> --- cmd/server/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 4e66d5d..b81adb9 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -135,7 +135,7 @@ func init() { serveCmd.Flags().Int("metrics-port", 9090, "Port for metrics endpoint") // Authentication configuration flags - serveCmd.Flags().String("tokensmith_url", "", "TokenSmith service URL for authentication") + serveCmd.Flags().String("tokensmith-url", "", "TokenSmith service URL for authentication") serveCmd.Flags().String("tokensmith-bootstrap-token", "", "Bootstrap token used to exchange HSM service tokens") serveCmd.Flags().String("tokensmith-target-service", "hsm", "Target service audience for HSM service token exchange") serveCmd.Flags().String("tokensmith-bootstrap-policy-scopes-hint", "", "Comma-separated scope hint from bootstrap token policy used for diagnostics only")