From 2fd7e3fd2cbd5141c9038813554280ff7067cb10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 00:21:41 +0000 Subject: [PATCH 1/7] Initial plan From 3a206e4db20195ef7b1a414f56da25c4ddf00b99 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 00:25:25 +0000 Subject: [PATCH 2/7] Add validation for unsupported config file extensions Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/config.go | 3 +- internal/config/viper.go | 34 ++++++++++++- internal/config/viper_test.go | 94 +++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 1b24695c..514fa43c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -27,7 +27,8 @@ func SetFileName(name string) { filename = name file.CreateEmptyIfNotExists(filename) - configureViper(filename) + err := configureViper(filename) + checkErr(err) } func SetFileNameForTest(t *testing.T) { diff --git a/internal/config/viper.go b/internal/config/viper.go index 9409fd8d..bef153a3 100644 --- a/internal/config/viper.go +++ b/internal/config/viper.go @@ -5,9 +5,12 @@ package config import ( "bytes" + "github.com/microsoft/go-sqlcmd/internal/localizer" "github.com/microsoft/go-sqlcmd/internal/pal" "github.com/spf13/viper" "gopkg.in/yaml.v2" + "path/filepath" + "strings" ) // Load loads the configuration from the file specified by the SetFileName() function. @@ -56,16 +59,45 @@ func GetConfigFileUsed() string { return viper.ConfigFileUsed() } +// validateConfigFileExtension checks if the config file has a supported extension. +// It allows .yaml, .yml, and no extension (for default sqlconfig file). +// Returns an error if the extension is not supported. +func validateConfigFileExtension(configFile string) error { + ext := strings.ToLower(filepath.Ext(configFile)) + + // Allow no extension (for default sqlconfig file) + if ext == "" { + return nil + } + + // Allow .yaml and .yml extensions + if ext == ".yaml" || ext == ".yml" { + return nil + } + + // Return error for unsupported extensions + return localizer.Errorf( + "Configuration files must use YAML format with .yaml or .yml extension.\n"+ + "The file '%s' has an unsupported extension '%s'.", + configFile, ext) +} + // configureViper initializes the Viper library with the given configuration file. // This function sets the configuration file type to "yaml" and sets the environment variable prefix to "SQLCMD". // It also sets the configuration file to use to the one provided as an argument to the function. // This function is intended to be called at the start of the application to configure Viper before any other code uses it. -func configureViper(configFile string) { +func configureViper(configFile string) error { if configFile == "" { panic("Must provide configFile") } + // Validate file extension + if err := validateConfigFileExtension(configFile); err != nil { + return err + } + viper.SetConfigType("yaml") viper.SetEnvPrefix("SQLCMD") viper.SetConfigFile(configFile) + return nil } diff --git a/internal/config/viper_test.go b/internal/config/viper_test.go index 3a606f6d..cb7ee28a 100644 --- a/internal/config/viper_test.go +++ b/internal/config/viper_test.go @@ -14,6 +14,100 @@ func Test_configureViper(t *testing.T) { }) } +func Test_validateConfigFileExtension(t *testing.T) { + tests := []struct { + name string + filename string + wantErr bool + }{ + { + name: "valid yaml extension", + filename: "config.yaml", + wantErr: false, + }, + { + name: "valid yml extension", + filename: "config.yml", + wantErr: false, + }, + { + name: "no extension (default sqlconfig)", + filename: "sqlconfig", + wantErr: false, + }, + { + name: "no extension with path", + filename: "/home/user/.sqlcmd/sqlconfig", + wantErr: false, + }, + { + name: "invalid txt extension", + filename: "config.txt", + wantErr: true, + }, + { + name: "invalid json extension", + filename: "config.json", + wantErr: true, + }, + { + name: "invalid xml extension", + filename: "config.xml", + wantErr: true, + }, + { + name: "uppercase YAML extension", + filename: "config.YAML", + wantErr: false, + }, + { + name: "uppercase YML extension", + filename: "config.YML", + wantErr: false, + }, + { + name: "mixed case yaml extension", + filename: "config.Yaml", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateConfigFileExtension(tt.filename) + if tt.wantErr { + assert.Error(t, err, "Expected error for filename: %s", tt.filename) + assert.Contains(t, err.Error(), "Configuration files must use YAML format") + } else { + assert.NoError(t, err, "Expected no error for filename: %s", tt.filename) + } + }) + } +} + +func Test_configureViper_withInvalidExtension(t *testing.T) { + err := configureViper("myconfig.txt") + assert.Error(t, err) + assert.Contains(t, err.Error(), "Configuration files must use YAML format") + assert.Contains(t, err.Error(), ".txt") +} + +func Test_configureViper_withValidExtensions(t *testing.T) { + testCases := []string{ + "config.yaml", + "config.yml", + "sqlconfig", + "/path/to/config.yaml", + } + + for _, filename := range testCases { + t.Run(filename, func(t *testing.T) { + err := configureViper(filename) + assert.NoError(t, err, "Expected no error for filename: %s", filename) + }) + } +} + func Test_Load(t *testing.T) { SetFileNameForTest(t) Clean() From 598dce0567a02f09099e7203241560c839b0c9f0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 00:26:14 +0000 Subject: [PATCH 3/7] Fix trailing whitespace in viper.go Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/viper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/config/viper.go b/internal/config/viper.go index bef153a3..e3a2554f 100644 --- a/internal/config/viper.go +++ b/internal/config/viper.go @@ -64,17 +64,17 @@ func GetConfigFileUsed() string { // Returns an error if the extension is not supported. func validateConfigFileExtension(configFile string) error { ext := strings.ToLower(filepath.Ext(configFile)) - + // Allow no extension (for default sqlconfig file) if ext == "" { return nil } - + // Allow .yaml and .yml extensions if ext == ".yaml" || ext == ".yml" { return nil } - + // Return error for unsupported extensions return localizer.Errorf( "Configuration files must use YAML format with .yaml or .yml extension.\n"+ From eeb651baf0005edec55ca628f7e0955f49be6aaa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 00:27:47 +0000 Subject: [PATCH 4/7] Organize imports in standard Go convention Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/viper.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/config/viper.go b/internal/config/viper.go index e3a2554f..dc4f69a9 100644 --- a/internal/config/viper.go +++ b/internal/config/viper.go @@ -5,12 +5,13 @@ package config import ( "bytes" + "path/filepath" + "strings" + "github.com/microsoft/go-sqlcmd/internal/localizer" "github.com/microsoft/go-sqlcmd/internal/pal" "github.com/spf13/viper" "gopkg.in/yaml.v2" - "path/filepath" - "strings" ) // Load loads the configuration from the file specified by the SetFileName() function. From 6e43792455831c492d335e5df1a241479612ceba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:34:23 +0000 Subject: [PATCH 5/7] Validate file extension before creating empty file Move validateConfigFileExtension call before file.CreateEmptyIfNotExists to avoid creating unnecessary files with invalid extensions on the filesystem. This addresses the review comment by ensuring validation happens first. Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/config.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 514fa43c..f52a8084 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -26,8 +26,12 @@ func SetFileName(name string) { filename = name + // Validate file extension before creating the file + err := validateConfigFileExtension(filename) + checkErr(err) + file.CreateEmptyIfNotExists(filename) - err := configureViper(filename) + err = configureViper(filename) checkErr(err) } From 9ae247fe83ee5b1065ca5dc191e244faf5082fb3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:48:05 +0000 Subject: [PATCH 6/7] Format error message as single line for consistency Changed multi-line error message to single-line format to maintain consistency with error message patterns throughout the codebase. Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/viper.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/config/viper.go b/internal/config/viper.go index dc4f69a9..5f19a24c 100644 --- a/internal/config/viper.go +++ b/internal/config/viper.go @@ -78,8 +78,7 @@ func validateConfigFileExtension(configFile string) error { // Return error for unsupported extensions return localizer.Errorf( - "Configuration files must use YAML format with .yaml or .yml extension.\n"+ - "The file '%s' has an unsupported extension '%s'.", + "Configuration files must use YAML format with .yaml or .yml extension. The file '%s' has an unsupported extension '%s'.", configFile, ext) } From 9a00d061bd031c533870e47990a47bbd4c163329 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Feb 2026 02:07:41 +0000 Subject: [PATCH 7/7] Add edge case tests for file extension validation Added test cases for edge scenarios: - Files with multiple dots (my.config.yaml, config.backup.txt) - Hidden files with extensions (.config.yaml, .config.txt) - Files with only dot and extension (.yaml, .yml) Total test cases increased from 10 to 19, improving coverage for various filename patterns that users might encounter. Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com> --- internal/config/viper_test.go | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/internal/config/viper_test.go b/internal/config/viper_test.go index cb7ee28a..f192d018 100644 --- a/internal/config/viper_test.go +++ b/internal/config/viper_test.go @@ -70,6 +70,51 @@ func Test_validateConfigFileExtension(t *testing.T) { filename: "config.Yaml", wantErr: false, }, + { + name: "multiple dots with valid extension", + filename: "my.config.yaml", + wantErr: false, + }, + { + name: "multiple dots with invalid extension", + filename: "my.config.txt", + wantErr: true, + }, + { + name: "backup file with valid extension", + filename: "config.backup.yaml", + wantErr: false, + }, + { + name: "backup file with invalid extension", + filename: "config.backup.txt", + wantErr: true, + }, + { + name: "hidden file with yaml extension", + filename: ".config.yaml", + wantErr: false, + }, + { + name: "hidden file with yml extension", + filename: ".config.yml", + wantErr: false, + }, + { + name: "hidden file with invalid extension", + filename: ".config.txt", + wantErr: true, + }, + { + name: "file with only dot and yaml", + filename: ".yaml", + wantErr: false, + }, + { + name: "file with only dot and yml", + filename: ".yml", + wantErr: false, + }, } for _, tt := range tests {