From 3c0854f7ea682b86a3b63540a6fade111edb0243 Mon Sep 17 00:00:00 2001 From: Ashutosh <11219262+ashutosh16@users.noreply.github.com> Date: Sun, 10 Apr 2022 07:55:19 -0700 Subject: [PATCH] chore: add permission check to argocd-cli (#9057) * add permission check to argocd-cli Signed-off-by: asingh51 * Retrigger CI pipeline Signed-off-by: asingh51 Co-authored-by: asingh51 --- cmd/argocd/commands/context_test.go | 6 ++- util/localconfig/localconfig.go | 20 ++++++- util/localconfig/localconfig_test.go | 78 ++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 4 deletions(-) diff --git a/cmd/argocd/commands/context_test.go b/cmd/argocd/commands/context_test.go index 70241d14554a4..b769fc1adb6f7 100644 --- a/cmd/argocd/commands/context_test.go +++ b/cmd/argocd/commands/context_test.go @@ -5,9 +5,9 @@ import ( "os" "testing" - "github.com/stretchr/testify/assert" - "github.com/argoproj/argo-cd/v2/util/localconfig" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const testConfig = `contexts: @@ -44,6 +44,8 @@ func TestContextDelete(t *testing.T) { err := ioutil.WriteFile(testConfigFilePath, []byte(testConfig), os.ModePerm) assert.NoError(t, err) + err = os.Chmod(testConfigFilePath, 0600) + require.NoError(t, err, "Could not change the file permission to 0600 %v", err) localConfig, err := localconfig.ReadLocalConfig(testConfigFilePath) assert.NoError(t, err) assert.Equal(t, localConfig.CurrentContext, "localhost:8080") diff --git a/util/localconfig/localconfig.go b/util/localconfig/localconfig.go index 7f7f4bf3c9aa0..7ca3da56ed6bc 100644 --- a/util/localconfig/localconfig.go +++ b/util/localconfig/localconfig.go @@ -2,13 +2,12 @@ package localconfig import ( "fmt" + "github.com/golang-jwt/jwt/v4" "os" "os/user" "path" "strings" - "github.com/golang-jwt/jwt/v4" - configUtil "github.com/argoproj/argo-cd/v2/util/config" ) @@ -79,6 +78,15 @@ func (u *User) Claims() (*jwt.RegisteredClaims, error) { func ReadLocalConfig(path string) (*LocalConfig, error) { var err error var config LocalConfig + + // check file permission only when argocd config exists + if fi, err := os.Stat(path); err == nil { + err = getFilePermission(fi) + if err != nil { + return nil, err + } + } + err = configUtil.UnmarshalLocalFile(path, &config) if os.IsNotExist(err) { return nil, nil @@ -303,3 +311,11 @@ func GetUsername(subject string) string { } return subject } + +func getFilePermission(fi os.FileInfo) error { + if fi.Mode().Perm() == 0600 || fi.Mode().Perm() == 0400 { + return nil + } + return fmt.Errorf("config file has incorrect permission flags:%s."+ + "change the file permission either to 0400 or 0600.", fi.Mode().Perm().String()) +} diff --git a/util/localconfig/localconfig_test.go b/util/localconfig/localconfig_test.go index 01b3f88fc72f4..59dae1159a398 100644 --- a/util/localconfig/localconfig_test.go +++ b/util/localconfig/localconfig_test.go @@ -1,6 +1,11 @@ package localconfig import ( + "fmt" + "github.com/stretchr/testify/require" + "os" + "path" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -11,3 +16,76 @@ func TestGetUsername(t *testing.T) { assert.Equal(t, "admin", GetUsername("admin")) assert.Equal(t, "", GetUsername("")) } + +func TestFilePermission(t *testing.T) { + dirPath := "testfolder/" + + err := os.MkdirAll(path.Dir(dirPath), 0700) + require.NoError(t, err, "Could not create argocd folder with 0700 permission: %v", err) + + t.Cleanup(func() { + err := os.RemoveAll(dirPath) + require.NoError(t, err, "Could not remove directory") + + }) + + for _, c := range []struct { + name string + testfile string + perm os.FileMode + expectedError error + }{ + { + name: "Test config file with permission 0700", + testfile: ".config_0700", + perm: 0700, + expectedError: fmt.Errorf("config file has incorrect permission flags:-rwx------.change the file permission either to 0400 or 0600."), + }, + { + name: "Test config file with permission 0777", + testfile: ".config_0777", + perm: 0777, + expectedError: fmt.Errorf("config file has incorrect permission flags:-rwxrwxrwx.change the file permission either to 0400 or 0600."), + }, + { + name: "Test config file with permission 0600", + testfile: ".config_0600", + perm: 0600, + expectedError: nil, + }, + { + name: "Test config file with permission 0400", + testfile: ".config_0400", + perm: 0400, + expectedError: nil, + }, + { + name: "Test config file with permission 0300", + testfile: ".config_0300", + perm: 0300, + expectedError: fmt.Errorf("config file has incorrect permission flags:--wx------.change the file permission either to 0400 or 0600."), + }, + } { + t.Run(c.name, func(t *testing.T) { + + filePath := filepath.Join(dirPath, c.testfile) + + f, err := os.Create(filePath) + require.NoError(t, err, "Could not write create config file: %v", err) + defer f.Close() + + err = f.Chmod(c.perm) + require.NoError(t, err, "Could not change the file permission to %s: %v", c.perm, err) + + fi, err := os.Stat(filePath) + require.NoError(t, err, "Could not access the fileinfo: %v", err) + + if err := getFilePermission(fi); err != nil { + assert.EqualError(t, err, c.expectedError.Error()) + } else { + require.Nil(t, err) + } + }) + } + +}