Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add permission check to argocd-cli #9057

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmd/argocd/commands/context_test.go
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
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
@@ -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)
}
})
}

}