diff --git a/pkg/cli/interactive.go b/pkg/cli/interactive.go index 00903a12df..fc11dd295f 100644 --- a/pkg/cli/interactive.go +++ b/pkg/cli/interactive.go @@ -1,11 +1,9 @@ package cli import ( - "errors" "fmt" "os" "path/filepath" - "regexp" "slices" "strings" @@ -17,9 +15,6 @@ import ( var interactiveLog = logger.New("cli:interactive") -// workflowNameRegex validates workflow names contain only alphanumeric characters, hyphens, and underscores -var workflowNameRegex = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) - // commonWorkflowNames contains common workflow name patterns for autocomplete suggestions var commonWorkflowNames = []string{ "issue-triage", @@ -34,12 +29,6 @@ var commonWorkflowNames = []string{ "documentation-check", } -// isValidWorkflowName checks if the provided workflow name contains only valid characters. -// Returns false for empty strings (which should be checked separately for a more specific error message). -func isValidWorkflowName(name string) bool { - return workflowNameRegex.MatchString(name) -} - // isAccessibleMode detects if accessibility mode should be enabled based on environment variables func isAccessibleMode() bool { return os.Getenv("TERM") == "dumb" || os.Getenv("NO_COLOR") != "" @@ -128,15 +117,7 @@ func (b *InteractiveWorkflowBuilder) promptForWorkflowName() error { Description("Enter a descriptive name for your workflow (e.g., 'issue-triage', 'code-review-helper')"). Suggestions(commonWorkflowNames). Value(&b.WorkflowName). - Validate(func(s string) error { - if s == "" { - return errors.New("workflow name cannot be empty") - } - if !isValidWorkflowName(s) { - return errors.New("workflow name must contain only alphanumeric characters, hyphens, and underscores") - } - return nil - }), + Validate(ValidateWorkflowName), ), ).WithAccessible(isAccessibleMode()) diff --git a/pkg/cli/interactive_test.go b/pkg/cli/interactive_test.go index aa369a0c59..d736d720e1 100644 --- a/pkg/cli/interactive_test.go +++ b/pkg/cli/interactive_test.go @@ -6,69 +6,70 @@ import ( "testing" ) -func TestIsValidWorkflowName(t *testing.T) { +func TestValidateWorkflowName_Integration(t *testing.T) { tests := []struct { - name string - input string - expected bool + name string + input string + expectError bool }{ { - name: "valid simple name", - input: "my-workflow", - expected: true, + name: "valid simple name", + input: "my-workflow", + expectError: false, }, { - name: "valid with underscores", - input: "my_workflow", - expected: true, + name: "valid with underscores", + input: "my_workflow", + expectError: false, }, { - name: "valid alphanumeric", - input: "workflow123", - expected: true, + name: "valid alphanumeric", + input: "workflow123", + expectError: false, }, { - name: "valid mixed", - input: "my-workflow_v2", - expected: true, + name: "valid mixed", + input: "my-workflow_v2", + expectError: false, }, { - name: "invalid with spaces", - input: "my workflow", - expected: false, + name: "invalid with spaces", + input: "my workflow", + expectError: true, }, { - name: "invalid with special chars", - input: "my@workflow!", - expected: false, + name: "invalid with special chars", + input: "my@workflow!", + expectError: true, }, { - name: "invalid with dots", - input: "my.workflow", - expected: false, + name: "invalid with dots", + input: "my.workflow", + expectError: true, }, { - name: "invalid with slashes", - input: "my/workflow", - expected: false, + name: "invalid with slashes", + input: "my/workflow", + expectError: true, }, { - name: "empty string", - input: "", - expected: false, + name: "empty string", + input: "", + expectError: true, }, { - name: "valid uppercase", - input: "MyWorkflow", - expected: true, + name: "valid uppercase", + input: "MyWorkflow", + expectError: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := isValidWorkflowName(tt.input) - if result != tt.expected { - t.Errorf("isValidWorkflowName(%q) = %v, want %v", tt.input, result, tt.expected) + err := ValidateWorkflowName(tt.input) + hasError := err != nil + if hasError != tt.expectError { + t.Errorf("ValidateWorkflowName(%q) error = %v, expectError %v", tt.input, err, tt.expectError) } }) } @@ -81,8 +82,8 @@ func TestCommonWorkflowNamesAreValid(t *testing.T) { } for _, name := range commonWorkflowNames { - if !isValidWorkflowName(name) { - t.Errorf("commonWorkflowNames contains invalid workflow name: %q", name) + if err := ValidateWorkflowName(name); err != nil { + t.Errorf("commonWorkflowNames contains invalid workflow name: %q (error: %v)", name, err) } } } diff --git a/pkg/cli/validators.go b/pkg/cli/validators.go new file mode 100644 index 0000000000..12dc5e5d16 --- /dev/null +++ b/pkg/cli/validators.go @@ -0,0 +1,21 @@ +package cli + +import ( + "errors" + "regexp" +) + +// workflowNameRegex validates workflow names contain only alphanumeric characters, hyphens, and underscores +var workflowNameRegex = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) + +// ValidateWorkflowName checks if the provided workflow name is valid. +// It ensures the name is not empty and contains only alphanumeric characters, hyphens, and underscores. +func ValidateWorkflowName(s string) error { + if s == "" { + return errors.New("workflow name cannot be empty") + } + if !workflowNameRegex.MatchString(s) { + return errors.New("workflow name must contain only alphanumeric characters, hyphens, and underscores") + } + return nil +} diff --git a/pkg/cli/validators_test.go b/pkg/cli/validators_test.go new file mode 100644 index 0000000000..5ce4555190 --- /dev/null +++ b/pkg/cli/validators_test.go @@ -0,0 +1,247 @@ +package cli + +import ( + "strings" + "testing" +) + +func TestValidateWorkflowName(t *testing.T) { + tests := []struct { + name string + input string + expectError bool + errorMsg string + }{ + { + name: "valid simple name", + input: "my-workflow", + expectError: false, + }, + { + name: "valid with underscores", + input: "my_workflow", + expectError: false, + }, + { + name: "valid alphanumeric", + input: "workflow123", + expectError: false, + }, + { + name: "valid mixed", + input: "my-workflow_v2", + expectError: false, + }, + { + name: "valid uppercase", + input: "MyWorkflow", + expectError: false, + }, + { + name: "valid all hyphens and underscores", + input: "my-workflow_test-123", + expectError: false, + }, + { + name: "empty string", + input: "", + expectError: true, + errorMsg: "workflow name cannot be empty", + }, + { + name: "invalid with spaces", + input: "my workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with special chars", + input: "my@workflow!", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with dots", + input: "my.workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with slashes", + input: "my/workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with parentheses", + input: "my(workflow)", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with brackets", + input: "my[workflow]", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with dollar sign", + input: "my$workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with percent sign", + input: "my%workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with hash", + input: "my#workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with asterisk", + input: "my*workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with ampersand", + input: "my&workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with plus", + input: "my+workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + { + name: "invalid with equals", + input: "my=workflow", + expectError: true, + errorMsg: "workflow name must contain only alphanumeric characters, hyphens, and underscores", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateWorkflowName(tt.input) + + if tt.expectError { + if err == nil { + t.Errorf("ValidateWorkflowName(%q) expected error but got nil", tt.input) + return + } + if !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("ValidateWorkflowName(%q) error = %q, want error containing %q", tt.input, err.Error(), tt.errorMsg) + } + } else { + if err != nil { + t.Errorf("ValidateWorkflowName(%q) unexpected error: %v", tt.input, err) + } + } + }) + } +} + +func TestValidateWorkflowName_EdgeCases(t *testing.T) { + tests := []struct { + name string + input string + expectError bool + }{ + { + name: "single character", + input: "a", + expectError: false, + }, + { + name: "single number", + input: "1", + expectError: false, + }, + { + name: "single hyphen", + input: "-", + expectError: false, + }, + { + name: "single underscore", + input: "_", + expectError: false, + }, + { + name: "very long valid name", + input: strings.Repeat("a", 100), + expectError: false, + }, + { + name: "starts with hyphen", + input: "-workflow", + expectError: false, + }, + { + name: "ends with hyphen", + input: "workflow-", + expectError: false, + }, + { + name: "starts with underscore", + input: "_workflow", + expectError: false, + }, + { + name: "ends with underscore", + input: "workflow_", + expectError: false, + }, + { + name: "starts with number", + input: "123workflow", + expectError: false, + }, + { + name: "multiple consecutive hyphens", + input: "my--workflow", + expectError: false, + }, + { + name: "multiple consecutive underscores", + input: "my__workflow", + expectError: false, + }, + { + name: "tab character", + input: "my\tworkflow", + expectError: true, + }, + { + name: "newline character", + input: "my\nworkflow", + expectError: true, + }, + { + name: "carriage return", + input: "my\rworkflow", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateWorkflowName(tt.input) + + if tt.expectError && err == nil { + t.Errorf("ValidateWorkflowName(%q) expected error but got nil", tt.input) + } + if !tt.expectError && err != nil { + t.Errorf("ValidateWorkflowName(%q) unexpected error: %v", tt.input, err) + } + }) + } +}