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: HOME with different permissions #2361

Merged
merged 25 commits into from
Sep 9, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
230 changes: 215 additions & 15 deletions pkg/docker/creds/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
}
}
5 changes: 2 additions & 3 deletions pkg/functions/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading