From db86bd2190b3e555aa5dde3f999bf712fccf397c Mon Sep 17 00:00:00 2001 From: reugn Date: Thu, 1 Jan 2026 17:28:25 +0200 Subject: [PATCH] feat(config): add configuration validation --- internal/cmd/lint.go | 6 +- internal/cmd/root.go | 7 +- internal/config/config.go | 38 +++++++- internal/config/config_test.go | 140 +++++++++++++++++++++++++++-- internal/config/format_settings.go | 16 ++++ internal/config/linter_config.go | 43 +++++++++ internal/config/style_settings.go | 33 +++++++ internal/config/upgrade_config.go | 19 ++++ 8 files changed, 289 insertions(+), 13 deletions(-) diff --git a/internal/cmd/lint.go b/internal/cmd/lint.go index 83bd7a5..c8e9a3d 100644 --- a/internal/cmd/lint.go +++ b/internal/cmd/lint.go @@ -67,7 +67,7 @@ func doLint(workflows []*workflow.Workflow, configFile string) int { issues, err := l.Lint() if err != nil { - fmt.Fprintf(os.Stderr, "failed to lint workflows: %v\n", err) + printError("failed to lint workflows: %v", err) return 1 } @@ -97,14 +97,14 @@ func doLint(workflows []*workflow.Workflow, configFile string) int { func doLintWithFix(l *linter.WorkflowLinter, issues []*linter.Issue, issuesExitCode int) int { // Apply fixes if err := l.Fix(); err != nil { - fmt.Fprintf(os.Stderr, "failed to fix workflows: %v\n", err) + printError("failed to fix workflows: %v", err) return 1 } // Re-lint to see what issues remain after fixing remainingIssues, err := l.Lint() if err != nil { - fmt.Fprintf(os.Stderr, "failed to re-lint workflows: %v\n", err) + printError("failed to re-lint workflows: %v", err) return 1 } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 1da4635..f70f3ec 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -16,11 +16,16 @@ var rootCmd = &cobra.Command{ func Execute() { if err := rootCmd.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) + printError("%v", err) os.Exit(1) } } +// printError prints a formatted error message to stderr. +func printError(format string, args ...any) { + fmt.Fprintf(os.Stderr, "✗ Error: "+format+"\n", args...) +} + func init() { rootCmd.AddCommand(initCmd) rootCmd.AddCommand(lintCmd) diff --git a/internal/config/config.go b/internal/config/config.go index 4ef5b2c..c499a48 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -25,12 +25,42 @@ type Config struct { Upgrade *UpgradeConfig `yaml:"upgrade,omitempty"` } +// Validate checks all configuration values for validity. +func (c *Config) Validate() error { + if err := c.Run.Validate(); err != nil { + return err + } + if err := c.Linters.Validate(); err != nil { + return err + } + if err := c.Upgrade.Validate(); err != nil { + return err + } + return nil +} + // RunConfig specifies general runtime settings. type RunConfig struct { Timeout string `yaml:"timeout"` // Duration string (e.g., "2m", "30s") IssuesExitCode int `yaml:"issues-exit-code"` // Exit code when issues are found (default: 1) } +// Validate checks RunConfig for invalid values. +func (r *RunConfig) Validate() error { + if r == nil { + return nil + } + if r.Timeout != "" { + if _, err := time.ParseDuration(r.Timeout); err != nil { + return fmt.Errorf("invalid timeout %q: %w", r.Timeout, err) + } + } + if r.IssuesExitCode != 0 && (r.IssuesExitCode < 1 || r.IssuesExitCode > 255) { + return fmt.Errorf("issues-exit-code must be between 1 and 255, got %d", r.IssuesExitCode) + } + return nil +} + const ( // DefaultTimeout is the default timeout for operations. DefaultTimeout = 5 * time.Minute @@ -41,7 +71,7 @@ const ( // GetTimeout returns the configured timeout duration. // Returns DefaultTimeout if not configured or invalid. func (c *Config) GetTimeout() time.Duration { - if c.Run == nil || c.Run.Timeout == "" { + if c == nil || c.Run == nil || c.Run.Timeout == "" { return DefaultTimeout } d, err := time.ParseDuration(c.Run.Timeout) @@ -55,7 +85,7 @@ func (c *Config) GetTimeout() time.Duration { // Returns DefaultIssuesExitCode (1) if not configured or invalid. // Exit codes must be in range 1-255; values outside this range return the default. func (c *Config) GetIssuesExitCode() int { - if c.Run == nil || c.Run.IssuesExitCode <= 0 || c.Run.IssuesExitCode > 255 { + if c == nil || c.Run == nil || c.Run.IssuesExitCode <= 0 || c.Run.IssuesExitCode > 255 { return DefaultIssuesExitCode } return c.Run.IssuesExitCode @@ -82,6 +112,10 @@ func LoadConfig(filename string) (*Config, error) { return nil, fmt.Errorf("failed to unmarshal config file: %w", err) } + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("invalid config: %w", err) + } + cfg.ensureDefaults() return &cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cfabde8..10fce7a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -17,8 +17,8 @@ func TestLoadConfig_NonExistent(t *testing.T) { if cfg.Linters == nil { t.Error("cfg.Linters is nil, want non-nil") } - if cfg.Linters.Default != "all" { - t.Errorf("cfg.Linters.Default = %q, want %q", cfg.Linters.Default, "all") + if cfg.Linters.Default != defaultLinterDefault { + t.Errorf("cfg.Linters.Default = %q, want %q", cfg.Linters.Default, defaultLinterDefault) } if cfg.Upgrade == nil { t.Error("cfg.Upgrade is nil, want non-nil") @@ -39,7 +39,7 @@ linters: enable: - permissions disable: - - security + - secrets settings: format: indent-width: 4 @@ -65,8 +65,8 @@ upgrade: if len(cfg.Linters.Enable) != 1 || cfg.Linters.Enable[0] != "permissions" { t.Errorf("cfg.Linters.Enable = %v, want [permissions]", cfg.Linters.Enable) } - if len(cfg.Linters.Disable) != 1 || cfg.Linters.Disable[0] != "security" { - t.Errorf("cfg.Linters.Disable = %v, want [security]", cfg.Linters.Disable) + if len(cfg.Linters.Disable) != 1 || cfg.Linters.Disable[0] != "secrets" { + t.Errorf("cfg.Linters.Disable = %v, want [secrets]", cfg.Linters.Disable) } // Check upgrade config @@ -630,8 +630,8 @@ func TestShouldUpdate(t *testing.T) { func TestFullDefaultLinterConfig(t *testing.T) { cfg := FullDefaultLinterConfig() - if cfg.Default != "all" { - t.Errorf("Default = %q, want %q", cfg.Default, "all") + if cfg.Default != defaultLinterDefault { + t.Errorf("Default = %q, want %q", cfg.Default, defaultLinterDefault) } // Should have all linters enabled @@ -693,3 +693,129 @@ func TestUpgradeConfig_EnsureDefaults(t *testing.T) { }) } } + +func TestConfig_Validate(t *testing.T) { + tests := []struct { + name string + config *Config + wantErr bool + }{ + { + name: "valid empty config", + config: &Config{}, + wantErr: false, + }, + { + name: "valid full config", + config: &Config{ + Run: &RunConfig{Timeout: "5m", IssuesExitCode: 2}, + Linters: &LinterConfig{Default: "all", Enable: []string{"versions"}}, + Upgrade: &UpgradeConfig{Version: "tag"}, + }, + wantErr: false, + }, + { + name: "invalid timeout", + config: &Config{Run: &RunConfig{Timeout: "invalid"}}, + wantErr: true, + }, + { + name: "invalid exit code too low", + config: &Config{Run: &RunConfig{IssuesExitCode: -1}}, + wantErr: true, + }, + { + name: "invalid exit code too high", + config: &Config{Run: &RunConfig{IssuesExitCode: 300}}, + wantErr: true, + }, + { + name: "invalid linter default", + config: &Config{Linters: &LinterConfig{Default: "invalid"}}, + wantErr: true, + }, + { + name: "unknown linter in enable", + config: &Config{Linters: &LinterConfig{Enable: []string{"unknown"}}}, + wantErr: true, + }, + { + name: "unknown linter in disable", + config: &Config{Linters: &LinterConfig{Disable: []string{"unknown"}}}, + wantErr: true, + }, + { + name: "invalid upgrade version format", + config: &Config{Upgrade: &UpgradeConfig{Version: "invalid"}}, + wantErr: true, + }, + { + name: "invalid format indent-width", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Format: &FormatSettings{IndentWidth: -1}}, + }}, + wantErr: true, + }, + { + name: "invalid format max-line-length", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Format: &FormatSettings{MaxLineLength: -1}}, + }}, + wantErr: true, + }, + { + name: "invalid style min-name-length negative", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Style: &StyleSettings{MinNameLength: -1}}, + }}, + wantErr: true, + }, + { + name: "invalid style max-name-length negative", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Style: &StyleSettings{MaxNameLength: -1}}, + }}, + wantErr: true, + }, + { + name: "invalid style max-run-lines negative", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Style: &StyleSettings{MaxRunLines: -1}}, + }}, + wantErr: true, + }, + { + name: "invalid style min > max name length", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Style: &StyleSettings{MinNameLength: 10, MaxNameLength: 5}}, + }}, + wantErr: true, + }, + { + name: "invalid naming convention", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{Style: &StyleSettings{NamingConvention: "invalid"}}, + }}, + wantErr: true, + }, + { + name: "valid settings", + config: &Config{Linters: &LinterConfig{ + Settings: &LinterSettings{ + Format: &FormatSettings{IndentWidth: 4, MaxLineLength: 100}, + Style: &StyleSettings{MinNameLength: 3, MaxNameLength: 50, NamingConvention: "title"}, + }, + }}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.config.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/config/format_settings.go b/internal/config/format_settings.go index f7952bc..f6adb95 100644 --- a/internal/config/format_settings.go +++ b/internal/config/format_settings.go @@ -1,5 +1,7 @@ package config +import "fmt" + const ( defaultIndentWidth = 2 defaultMaxLineLength = 120 @@ -13,6 +15,20 @@ type FormatSettings struct { MaxLineLength int `yaml:"max-line-length"` } +// Validate checks FormatSettings for invalid values. +func (f *FormatSettings) Validate() error { + if f == nil { + return nil + } + if f.IndentWidth < 0 { + return fmt.Errorf("format.indent-width must be non-negative, got %d", f.IndentWidth) + } + if f.MaxLineLength < 0 { + return fmt.Errorf("format.max-line-length must be non-negative, got %d", f.MaxLineLength) + } + return nil +} + // DefaultFormatSettings returns the default format linter settings. func DefaultFormatSettings() *FormatSettings { return &FormatSettings{ diff --git a/internal/config/linter_config.go b/internal/config/linter_config.go index e050446..5b24e26 100644 --- a/internal/config/linter_config.go +++ b/internal/config/linter_config.go @@ -1,5 +1,10 @@ package config +import ( + "fmt" + "slices" +) + const defaultLinterDefault = "all" // LinterConfig specifies which linters to enable and their behavior. @@ -11,12 +16,50 @@ type LinterConfig struct { Settings *LinterSettings `yaml:"settings,omitempty"` // Per-linter settings } +// Validate checks LinterConfig for invalid values. +func (l *LinterConfig) Validate() error { + if l == nil { + return nil + } + if l.Default != "" && l.Default != "all" && l.Default != "none" { + return fmt.Errorf("linters.default must be \"all\" or \"none\", got %q", l.Default) + } + for _, name := range l.Enable { + if !slices.Contains(allLinters, name) { + return fmt.Errorf("unknown linter %q in linters.enable", name) + } + } + for _, name := range l.Disable { + if !slices.Contains(allLinters, name) { + return fmt.Errorf("unknown linter %q in linters.disable", name) + } + } + if err := l.Settings.Validate(); err != nil { + return err + } + return nil +} + // LinterSettings contains per-linter configuration. type LinterSettings struct { Format *FormatSettings `yaml:"format,omitempty"` Style *StyleSettings `yaml:"style,omitempty"` } +// Validate checks LinterSettings for invalid values. +func (s *LinterSettings) Validate() error { + if s == nil { + return nil + } + if err := s.Format.Validate(); err != nil { + return err + } + if err := s.Style.Validate(); err != nil { + return err + } + return nil +} + // DefaultLinterConfig returns a minimal LinterConfig with default values. func DefaultLinterConfig() *LinterConfig { return &LinterConfig{ diff --git a/internal/config/style_settings.go b/internal/config/style_settings.go index b1cee42..378d4df 100644 --- a/internal/config/style_settings.go +++ b/internal/config/style_settings.go @@ -1,11 +1,19 @@ package config +import ( + "fmt" + "slices" +) + const ( defaultMinNameLength = 3 defaultMaxNameLength = 50 defaultMaxRunLines = 0 // 0 means disabled ) +// Valid naming conventions. +var validNamingConventions = []string{"title", "sentence"} + // StyleSettings contains settings for the style linter. type StyleSettings struct { // MinNameLength is the minimum allowed characters for names (default: 3) @@ -25,6 +33,31 @@ type StyleSettings struct { MaxRunLines int `yaml:"max-run-lines"` } +// Validate checks StyleSettings for invalid values. +func (s *StyleSettings) Validate() error { + if s == nil { + return nil + } + if s.MinNameLength < 0 { + return fmt.Errorf("style.min-name-length must be non-negative, got %d", s.MinNameLength) + } + if s.MaxNameLength < 0 { + return fmt.Errorf("style.max-name-length must be non-negative, got %d", s.MaxNameLength) + } + if s.MinNameLength > 0 && s.MaxNameLength > 0 && s.MinNameLength > s.MaxNameLength { + return fmt.Errorf("style.min-name-length (%d) cannot be greater than max-name-length (%d)", + s.MinNameLength, s.MaxNameLength) + } + if s.NamingConvention != "" && !slices.Contains(validNamingConventions, s.NamingConvention) { + return fmt.Errorf("style.naming-convention must be one of %v, got %q", + validNamingConventions, s.NamingConvention) + } + if s.MaxRunLines < 0 { + return fmt.Errorf("style.max-run-lines must be non-negative, got %d", s.MaxRunLines) + } + return nil +} + // DefaultStyleSettings returns the default style linter settings. func DefaultStyleSettings() *StyleSettings { return &StyleSettings{ diff --git a/internal/config/upgrade_config.go b/internal/config/upgrade_config.go index e420f66..478ad1e 100644 --- a/internal/config/upgrade_config.go +++ b/internal/config/upgrade_config.go @@ -1,10 +1,18 @@ package config +import ( + "fmt" + "slices" +) + const ( defaultVersionPattern = "^1.0.0" defaultUpgradeVersion = "tag" ) +// Valid version formats for upgrades. +var validVersionFormats = []string{"tag", "hash", "major"} + // UpgradeConfig specifies settings for the upgrade command. type UpgradeConfig struct { Actions map[string]ActionConfig `yaml:"actions"` @@ -16,6 +24,17 @@ type ActionConfig struct { Version string `yaml:"version"` } +// Validate checks UpgradeConfig for invalid values. +func (u *UpgradeConfig) Validate() error { + if u == nil { + return nil + } + if u.Version != "" && !slices.Contains(validVersionFormats, u.Version) { + return fmt.Errorf("upgrade.version must be one of %v, got %q", validVersionFormats, u.Version) + } + return nil +} + // DefaultUpgradeConfig returns an UpgradeConfig with default values. func DefaultUpgradeConfig() *UpgradeConfig { return &UpgradeConfig{