Skip to content

Commit

Permalink
chore: add permission check to argocd-cli (#9057)
Browse files Browse the repository at this point in the history
* add permission check to argocd-cli

Signed-off-by: asingh51 <Ashutosh_Singh@intuit.com>

* Retrigger CI pipeline

Signed-off-by: asingh51 <Ashutosh_Singh@intuit.com>

Co-authored-by: asingh51 <Ashutosh_Singh@intuit.com>
  • Loading branch information
ashutosh16 and asingh51 authored Apr 10, 2022
1 parent 60eb2af commit 3c0854f
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 4 deletions.
6 changes: 4 additions & 2 deletions cmd/argocd/commands/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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")
Expand Down
20 changes: 18 additions & 2 deletions util/localconfig/localconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
78 changes: 78 additions & 0 deletions util/localconfig/localconfig_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package localconfig

import (
"fmt"
"github.com/stretchr/testify/require"
"os"
"path"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -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)
}
})
}

}

0 comments on commit 3c0854f

Please sign in to comment.