diff --git a/scw/client_option_test.go b/scw/client_option_test.go index 6041defc..501fc1bb 100644 --- a/scw/client_option_test.go +++ b/scw/client_option_test.go @@ -240,7 +240,7 @@ func TestCombinedClientOptions(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // set up env and config file(s) - setEnv(t, test.env, test.files, dir) + setEnv(t, test.env, test.files, defaultConfigPermission, dir) test.expectedError = strings.ReplaceAll(test.expectedError, "{HOME}", dir) // remove config file(s) diff --git a/scw/config.go b/scw/config.go index af0995df..f69dd8aa 100644 --- a/scw/config.go +++ b/scw/config.go @@ -3,6 +3,8 @@ package scw import ( "bytes" goerrors "errors" + "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -200,7 +202,7 @@ func LoadConfig() (*Config, error) { configPath = strings.TrimSuffix(configPath, ".yaml") + ".yml" cfgYml, errYml := LoadConfigFromPath(configPath) // If .yml config is not found, return first error when reading .yaml - if errYml == nil || (errYml != nil && !goerrors.As(errYml, &configNotFoundError)) { + if errYml == nil || !goerrors.As(errYml, &configNotFoundError) { return cfgYml, errYml } } @@ -211,7 +213,7 @@ func LoadConfig() (*Config, error) { // LoadConfigFromPath read the config from the given path. func LoadConfigFromPath(path string) (*Config, error) { - _, err := os.Stat(path) + fileInfo, err := os.Stat(path) if os.IsNotExist(err) { return nil, configFileNotFound(path) } @@ -219,6 +221,11 @@ func LoadConfigFromPath(path string) (*Config, error) { return nil, err } + if permsAreTooPermissive(fileInfo.Mode().Perm()) { + fmt.Printf("WARNING: Scaleway configuration file permissions are too permissive. That is insecure.\n"+ + "You can fix it with the command 'chmod 0600 %s'\n", path) + } + file, err := os.ReadFile(path) if err != nil { return nil, errors.Wrap(err, "cannot read config file") @@ -232,6 +239,20 @@ func LoadConfigFromPath(path string) (*Config, error) { return confV2, nil } +func permsAreTooPermissive(perms fs.FileMode) bool { + if perms > defaultConfigPermission { + return true + } + + strPerms := perms.String() + if strPerms[4:7] != "---" || + strPerms[7:9] != "---" { + return true + } + + return false +} + // GetProfile returns the profile corresponding to the given profile name. func (c *Config) GetProfile(profileName string) (*Profile, error) { if profileName == "" { diff --git a/scw/config_test.go b/scw/config_test.go index fa269e94..fcc76fdd 100644 --- a/scw/config_test.go +++ b/scw/config_test.go @@ -1,6 +1,9 @@ package scw import ( + "bytes" + "fmt" + "io" "os" "path/filepath" "strings" @@ -230,7 +233,7 @@ func TestSaveConfig(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // set up env and config file(s) - setEnv(t, test.env, test.files, dir) + setEnv(t, test.env, test.files, defaultConfigPermission, dir) // remove config file(s) defer cleanEnv(t, test.files, dir) @@ -252,12 +255,13 @@ func TestSaveConfig(t *testing.T) { } } -// TestLoadConfig tests config getters return correct values +// TestLoadProfileAndActiveProfile tests config getters return correct values func TestLoadProfileAndActiveProfile(t *testing.T) { tests := []struct { name string env map[string]string files map[string]string + perms os.FileMode expectedError string expectedAccessKey *string @@ -269,6 +273,7 @@ func TestLoadProfileAndActiveProfile(t *testing.T) { expectedDefaultProjectID *string expectedDefaultRegion *string expectedDefaultZone *string + expectedOutput string }{ // no env variables { @@ -406,6 +411,85 @@ func TestLoadProfileAndActiveProfile(t *testing.T) { expectedDefaultProjectID: s(v2ValidDefaultProjectID), expectedDefaultRegion: s(v2ValidDefaultRegion), }, + { + name: "Read config.yml too permissive", + env: map[string]string{ + "HOME": "{HOME}", + }, + files: map[string]string{ + ".config/scw/config.yml": v2SimpleValidConfigFile, + }, + perms: 0o700, + expectedAccessKey: s(v2ValidAccessKey), + expectedSecretKey: s(v2ValidSecretKey), + expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID), + expectedDefaultProjectID: s(v2ValidDefaultProjectID), + expectedDefaultRegion: s(v2ValidDefaultRegion), + expectedOutput: `WARNING: Scaleway configuration file permissions are too permissive. That is insecure.` + ` +You can fix it with the command 'chmod 0600 {HOME}/.config/scw/config.yml'`, + }, + { + name: "Read config.yml too permissive", + env: map[string]string{ + "HOME": "{HOME}", + }, + files: map[string]string{ + ".config/scw/config.yml": v2SimpleValidConfigFile, + }, + perms: 0o650, + expectedAccessKey: s(v2ValidAccessKey), + expectedSecretKey: s(v2ValidSecretKey), + expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID), + expectedDefaultProjectID: s(v2ValidDefaultProjectID), + expectedDefaultRegion: s(v2ValidDefaultRegion), + expectedOutput: `WARNING: Scaleway configuration file permissions are too permissive. That is insecure.` + ` +You can fix it with the command 'chmod 0600 {HOME}/.config/scw/config.yml'`, + }, + { + name: "Read config.yml too permissive", + env: map[string]string{ + "HOME": "{HOME}", + }, + files: map[string]string{ + ".config/scw/config.yml": v2SimpleValidConfigFile, + }, + perms: 0o477, + expectedAccessKey: s(v2ValidAccessKey), + expectedSecretKey: s(v2ValidSecretKey), + expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID), + expectedDefaultProjectID: s(v2ValidDefaultProjectID), + expectedDefaultRegion: s(v2ValidDefaultRegion), + expectedOutput: `WARNING: Scaleway configuration file permissions are too permissive. That is insecure.` + ` +You can fix it with the command 'chmod 0600 {HOME}/.config/scw/config.yml'`, + }, + { + name: "Read config.yml too permissive", + env: map[string]string{ + "HOME": "{HOME}", + }, + files: map[string]string{ + ".config/scw/config.yml": v2SimpleValidConfigFile, + }, + perms: 0o605, + expectedAccessKey: s(v2ValidAccessKey), + expectedSecretKey: s(v2ValidSecretKey), + expectedDefaultOrganizationID: s(v2ValidDefaultOrganizationID), + expectedDefaultProjectID: s(v2ValidDefaultProjectID), + expectedDefaultRegion: s(v2ValidDefaultRegion), + expectedOutput: `WARNING: Scaleway configuration file permissions are too permissive. That is insecure.` + ` +You can fix it with the command 'chmod 0600 {HOME}/.config/scw/config.yml'`, + }, + { + name: "Read config.yml too restrictive", + env: map[string]string{ + "HOME": "{HOME}", + }, + files: map[string]string{ + ".config/scw/config.yml": v2SimpleValidConfigFile, + }, + perms: 0o300, + expectedError: "scaleway-sdk-go: cannot read config file: open {HOME}/.config/scw/config.yml: permission denied", + }, } // create home dir @@ -417,13 +501,24 @@ func TestLoadProfileAndActiveProfile(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // set up env and config file(s) - setEnv(t, test.env, test.files, dir) + setEnv(t, test.env, test.files, test.perms, dir) test.expectedError = strings.ReplaceAll(test.expectedError, "{HOME}", dir) + test.expectedOutput = strings.ReplaceAll(test.expectedOutput, "{HOME}", dir) // remove config file(s) defer cleanEnv(t, test.files, dir) + // Temporarily capturing stdout + originalStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + config, err := LoadConfig() + + // Giving back stdout + w.Close() + os.Stdout = originalStdout + if test.expectedError == "" { testhelpers.AssertNoError(t, err) p, err := config.GetActiveProfile() @@ -442,6 +537,16 @@ func TestLoadProfileAndActiveProfile(t *testing.T) { } else { testhelpers.Equals(t, test.expectedError, err.Error()) } + + // In both cases, read captured stdout + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + testhelpers.AssertNoError(t, err) + testhelpers.Assert( + t, + strings.Contains(buf.String(), test.expectedOutput), + fmt.Sprintf("expected\n%s\nto contain\n%s", buf.String(), test.expectedOutput), + ) }) } } @@ -523,7 +628,7 @@ func cleanEnv(t *testing.T, files map[string]string, homeDir string) { } } -func setEnv(t *testing.T, env, files map[string]string, homeDir string) { +func setEnv(t *testing.T, env, files map[string]string, perms os.FileMode, homeDir string) { t.Helper() os.Clearenv() for key, value := range env { @@ -531,10 +636,14 @@ func setEnv(t *testing.T, env, files map[string]string, homeDir string) { testhelpers.AssertNoError(t, os.Setenv(key, value)) } + if perms == 0 { + perms = defaultConfigPermission + } + for path, content := range files { targetPath := filepath.Join(homeDir, path) testhelpers.AssertNoError(t, os.MkdirAll(filepath.Dir(targetPath), 0o700)) - testhelpers.AssertNoError(t, os.WriteFile(targetPath, []byte(content), defaultConfigPermission)) + testhelpers.AssertNoError(t, os.WriteFile(targetPath, []byte(content), perms)) } } diff --git a/scw/env_test.go b/scw/env_test.go index fb3e77da..1d37a1c0 100644 --- a/scw/env_test.go +++ b/scw/env_test.go @@ -82,7 +82,7 @@ func TestLoadEnvProfile(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // set up env and config file(s) - setEnv(t, test.env, nil, dir) + setEnv(t, test.env, nil, defaultConfigPermission, dir) // remove config file(s) defer cleanEnv(t, nil, dir) diff --git a/scw/load_config_test.go b/scw/load_config_test.go index 7f3d9a31..cf511bf1 100644 --- a/scw/load_config_test.go +++ b/scw/load_config_test.go @@ -117,7 +117,7 @@ func TestLoad(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { // set up env and config file(s) - setEnv(t, test.env, test.files, dir) + setEnv(t, test.env, test.files, defaultConfigPermission, dir) test.expectedError = strings.ReplaceAll(test.expectedError, "{HOME}", dir) // remove config file(s)