diff --git a/cmd/root.go b/cmd/root.go index 254104df30..6ad99c8000 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -64,6 +64,11 @@ Learn more about Knative at: https://knative.dev`, cfg.Name), viper.SetEnvPrefix("func") // ensure that all have the prefix viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) + // check if permissions for FUNC HOME are sufficient; warn if otherwise + cp := config.File() + if _, err := os.ReadFile(cp); os.IsPermission(err) { + fmt.Fprintf(os.Stderr, "Warning: Insufficient permissions to read config file at '%s' - continuing without it\n", cp) + } // Client // Use the provided ClientFactory or default to NewClient newClient := cfg.NewClient diff --git a/pkg/config/config.go b/pkg/config/config.go index e357b733d4..f5f5312f6a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -81,8 +81,11 @@ func NewDefault() (cfg Global, err error) { cp := File() bb, err := os.ReadFile(cp) if err != nil { - if os.IsNotExist(err) { - err = nil // config file is not required + // config file is not required + // permissions warning printed in cmd/root.go as to not spam here + // TODO: gauron99 - review the whole process for simplification + if os.IsNotExist(err) || os.IsPermission(err) { + err = nil } return } diff --git a/pkg/docker/creds/credentials_test.go b/pkg/docker/creds/credentials_test.go index 7522bde3fd..c1227d152e 100644 --- a/pkg/docker/creds/credentials_test.go +++ b/pkg/docker/creds/credentials_test.go @@ -549,7 +549,8 @@ func TestCredentialsProviderSavingFromUserInput(t *testing.T) { } } -// TestCredentialsWithoutHome tests different scenarios when HOME is not set +// TestCredentialsWithoutHome ensures that credentialProvider works when HOME is +// not set or config is empty func TestCredentialsWithoutHome(t *testing.T) { type args struct { promptUser creds.CredentialsCallback @@ -585,30 +586,31 @@ func TestCredentialsWithoutHome(t *testing.T) { }, } + // reset HOME to the original value after tests since they may change it + defer func() { + os.Setenv("HOME", homeTempDir) + }() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { resetHomeDir(t) - if tt.args.setUpEnv != nil { - tt.args.setUpEnv(t) - } - - // prepare config path for credentials provider - var configPath string + // set up HOME if tt.testHomePathEmpty { - configPath = "" + os.Unsetenv("HOME") } else { - configPath = testConfigPath(t) + os.Setenv("HOME", homeTempDir) } - credentialsProvider := creds.NewCredentialsProvider( - configPath, + testConfigPath(t), creds.WithPromptForCredentials(tt.args.promptUser), creds.WithVerifyCredentials(tt.args.verifyCredentials), ) got, err := credentialsProvider(context.Background(), tt.args.registry+"/someorg/someimage:sometag") + + // ASSERT if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("%v", err) return } if !reflect.DeepEqual(got, tt.want) { @@ -618,6 +620,166 @@ func TestCredentialsWithoutHome(t *testing.T) { } } +// TestCredentialsHomePermissions tests whether the credentials provider +// works in scenarios where HOME has different permissions +func TestCredentialsHomePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skip windows perms for this test until windows perms are added") + } + + if os.Getenv("GITHUB_ACTION") == "" { + // skip for prow because its running as root + t.Skip() + } + + type args struct { + promptUser creds.CredentialsCallback + verifyCredentials creds.VerifyCredentialsCallback + registry string + setUpEnv setUpEnv + } + tests := []struct { + name string + perms os.FileMode + args args + expPermsDeniedErr bool + want Credentials + }{ + { + name: "home with 0000 permissions (no perms)", + perms: 0000, + args: args{ + promptUser: pwdCbkThatShallNotBeCalled(t), + setUpEnv: setHomeWithPermissions(0000), + }, + expPermsDeniedErr: true, + }, + { + name: "home with 0333 permissions (write-execute only)", + perms: 0333, + args: args{ + + promptUser: pwdCbkThatShallNotBeCalled(t), + verifyCredentials: correctVerifyCbk, + registry: "docker.io", + setUpEnv: all( + withPopulatedDockerAuthConfig, + setUpMockHelper("docker-credential-mock", newInMemoryHelper()), + setHomeWithPermissions(0333)), + }, + + expPermsDeniedErr: false, + want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd}, + }, + { + name: "home with 0444 permissions (read-only)", + perms: 0444, + args: args{ + promptUser: pwdCbkThatShallNotBeCalled(t), + setUpEnv: setHomeWithPermissions(0444)}, + expPermsDeniedErr: true, + }, + { + name: "home with 0555 permissions (read-execute-only)", + perms: 0555, + args: args{ + promptUser: pwdCbkThatShallNotBeCalled(t), + setUpEnv: setHomeWithPermissions(0555), + }, + expPermsDeniedErr: true, + }, + { + name: "home with 0666 permissions (read-write-execute)", + perms: 0666, + args: args{ + promptUser: pwdCbkThatShallNotBeCalled(t), + setUpEnv: setHomeWithPermissions(0666), + }, + expPermsDeniedErr: true, + }, + { + name: "home with 0777 permissions (full access)", + perms: 0777, + + args: args{ + promptUser: pwdCbkThatShallNotBeCalled(t), + verifyCredentials: correctVerifyCbk, + registry: "docker.io", + setUpEnv: all( + withPopulatedDockerAuthConfig, + setUpMockHelper("docker-credential-mock", newInMemoryHelper()), + setHomeWithPermissions(0777), + ), + }, + expPermsDeniedErr: false, + want: Credentials{Username: dockerIoUser, Password: dockerIoUserPwd}, + }, + } + + // return HOME dir into its original state + defer func() { + resetHomePermissions(t) //reset home permissions to 0700 + }() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + resetHomePermissions(t) //needs to be reset so that dir can be removed + resetHomeDir(t) + + if tt.args.setUpEnv != nil { + tt.args.setUpEnv(t) + } + + // try to create HOME/.config/func + _, err := testConfigPathError(t) + if err != nil { // If error was returned + if os.IsPermission(err) { // and its a permission error + if !tt.expPermsDeniedErr { // but it wasnt expected + t.Fatalf("didnt expect permissions denied error, but got: %s", err) + } + + } else { // and it wasnt permission error + t.Fatalf("got unexpected error: %v", err) + } + } else { // Else no error was returned + if tt.expPermsDeniedErr { // but it was expected + t.Fatal("expected permissions denied error, but got none") + } + } + + // if permissions were not denied, try to create Provider + if !tt.expPermsDeniedErr { + + // try to stat HOME for permissions + info, err := os.Stat(os.Getenv(testHomeEnvName())) + if err != nil { + t.Fatalf("failed to stat HOME: %s", err) + } + + if info.Mode().Perm() != tt.perms { + t.Errorf("expected permissions '%v', got '%v'", tt.perms, info.Mode().Perm()) + } + credentialsProvider := creds.NewCredentialsProvider( + testConfigPath(t), + creds.WithPromptForCredentials(tt.args.promptUser), + creds.WithVerifyCredentials(tt.args.verifyCredentials), + ) + + got, err := credentialsProvider(context.Background(), tt.args.registry+"/someorg/someimage:sometag") + if err != nil { + t.Errorf("%v", err) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("got: %v, want: %v", got, tt.want) + } + + } + }) + } +} + // ********************** helper functions below **************************** \\ func resetHomeDir(t *testing.T) { @@ -630,6 +792,13 @@ func resetHomeDir(t *testing.T) { } } +// resetHomePermissions resets the HOME perms to 0700 (same as resetHomeDir(t)) +func resetHomePermissions(t *testing.T) { + if err := os.Chmod(homeTempDir, 0700); err != nil { + t.Fatal(err) + } +} + type setUpEnv = func(t *testing.T) func withPopulatedDockerAuthConfig(t *testing.T) { @@ -743,13 +912,25 @@ func testHomeEnvName() string { func testConfigPath(t *testing.T) string { t.Helper() home := os.Getenv(testHomeEnvName()) - configPath := filepath.Join(home, ".config", "func") - if err := os.MkdirAll(configPath, os.ModePerm); err != nil { - t.Fatal(err) + var configPath string + if home != "" { // if HOME is not set, don't create config dir + configPath = filepath.Join(home, ".config", "func") + if err := os.MkdirAll(configPath, os.ModePerm); err != nil { + t.Fatal(err) + } } return configPath } +// testConfigPathError tries to create a config dir in HOME/.config/func. +// Compared to testConfigPath, this returns the path AND error instead of failing +func testConfigPathError(t *testing.T) (string, error) { + t.Helper() + home := os.Getenv(testHomeEnvName()) + configPath := filepath.Join(home, ".config", "func") + return configPath, os.MkdirAll(configPath, os.ModePerm) +} + func handlerForCredHelper(t *testing.T, credHelper credentials.Helper) http.Handler { return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { defer request.Body.Close() @@ -981,3 +1162,22 @@ func setEmptyHome(t *testing.T) { t.Setenv("HOME", "") t.Setenv("XDG_CONFIG_HOME", "") } + +// setHomeWithPermissions sets home dir to specified permissions +func setHomeWithPermissions(perm os.FileMode) func(t *testing.T) { + return func(t *testing.T) { + t.Helper() + homeDir := os.Getenv("HOME") + + // if home is empty, nothing to do + if homeDir == "" { + t.Fatal("home dir is empty, cant set perms") + } + + fmt.Printf("setting permissions (%v) on home dir: %s\n", perm, homeDir) + err := os.Chmod(homeDir, perm) + if err != nil { + t.Fatalf("failed to set permissions on home dir: %s", err) + } + } +} diff --git a/pkg/functions/repositories.go b/pkg/functions/repositories.go index 3288a84dfd..a48038f341 100644 --- a/pkg/functions/repositories.go +++ b/pkg/functions/repositories.go @@ -100,9 +100,8 @@ func (r *Repositories) All() (repos []Repository, err error) { if r.path == "" { return } - - // Return empty if path does not exist - if _, err = os.Stat(r.path); os.IsNotExist(err) { + // Return empty if path does not exist or insufficient permissions + if _, err = os.Stat(r.path); os.IsNotExist(err) || os.IsPermission(err) { return repos, nil }