diff --git a/integration/dockerfiles/Dockerfile_test_user_without_grp b/integration/dockerfiles/Dockerfile_test_user_home similarity index 58% rename from integration/dockerfiles/Dockerfile_test_user_without_grp rename to integration/dockerfiles/Dockerfile_test_user_home index 5c1f955e9..2bab30dcd 100644 --- a/integration/dockerfiles/Dockerfile_test_user_without_grp +++ b/integration/dockerfiles/Dockerfile_test_user_home @@ -1,4 +1,4 @@ -# Copyright 2018 Google, Inc. All rights reserved. +# Copyright 2020 Google, Inc. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -13,7 +13,23 @@ # limitations under the License. FROM debian:9.11 -RUN groupadd testgroup && \ - useradd --create-home --gid testgroup alice -USER alice \ No newline at end of file +# default values for the root user. +RUN touch $HOME/hello + +# testuser1 with the default home created by useradd. + +RUN groupadd testgroup && \ + useradd --create-home --gid testgroup alice && \ + useradd --create-home --uid 1111 --home-dir /home/john --gid testgroup bob + +USER alice:testgroup +RUN touch $HOME/hello + +USER bob +RUN touch $HOME/hello + +USER root + +USER 1111 +RUN touch $HOME/world diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 9da6ddeeb..b60f185b3 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -40,7 +40,8 @@ type RunCommand struct { // for testing var ( - userLookup = user.Lookup + userLookup = user.Lookup + userLookupID = user.LookupId ) func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { @@ -68,16 +69,29 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui cmd.Stderr = os.Stderr replacementEnvs := buildArgs.ReplacementEnvs(config.Env) cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - var userStr string + + u := config.User + userAndGroup := strings.Split(u, ":") + userStr, err := util.ResolveEnvironmentReplacement(userAndGroup[0], replacementEnvs, false) + if err != nil { + return errors.Wrapf(err, "resolving user %s", userAndGroup[0]) + } + // If specified, run the command as a specific user - if config.User != "" { - uid, gid, err := util.GetUIDAndGIDFromString(config.User, true) + if userStr != "" { + uid, gid, err := util.GetUIDAndGIDFromString(userStr, true) if err != nil { return err } cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid} } - cmd.Env = addDefaultHOME(userStr, replacementEnvs) + + env, err := addDefaultHOME(userStr, replacementEnvs) + if err != nil { + return errors.Wrap(err, "adding default HOME variable") + } + + cmd.Env = env if err := cmd.Start(); err != nil { return errors.Wrap(err, "starting command") @@ -100,32 +114,31 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui } // addDefaultHOME adds the default value for HOME if it isn't already set -func addDefaultHOME(u string, envs []string) []string { +func addDefaultHOME(u string, envs []string) ([]string, error) { for _, env := range envs { split := strings.SplitN(env, "=", 2) if split[0] == constants.HOME { - return envs + return envs, nil } } // If user isn't set, set default value of HOME if u == "" || u == constants.RootUser { - return append(envs, fmt.Sprintf("%s=%s", constants.HOME, constants.DefaultHOMEValue)) + return append(envs, fmt.Sprintf("%s=%s", constants.HOME, constants.DefaultHOMEValue)), nil } // If user is set to username, set value of HOME to /home/${user} // Otherwise the user is set to uid and HOME is / - home := "/" userObj, err := userLookup(u) - if err == nil { - if userObj.HomeDir != "" { - home = userObj.HomeDir + if err != nil { + if uo, e := userLookupID(u); e == nil { + userObj = uo } else { - home = fmt.Sprintf("/home/%s", userObj.Username) + return nil, err } } - return append(envs, fmt.Sprintf("%s=%s", constants.HOME, home)) + return append(envs, fmt.Sprintf("%s=%s", constants.HOME, userObj.HomeDir)), nil } // String returns some information about the command for the image config diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index 7e4984eb1..f6accc3bb 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -18,6 +18,7 @@ package commands import ( "archive/tar" "bytes" + "errors" "io" "io/ioutil" "log" @@ -33,11 +34,13 @@ import ( func Test_addDefaultHOME(t *testing.T) { tests := []struct { - name string - user string - mockUser *user.User - initial []string - expected []string + name string + user string + mockUser *user.User + lookupError error + mockUserID *user.User + initial []string + expected []string }{ { name: "HOME already set", @@ -78,31 +81,19 @@ func Test_addDefaultHOME(t *testing.T) { }, }, { - name: "HOME not set and user set", - user: "www-add", - mockUser: &user.User{ - Username: "www-add", + name: "USER is set using the UID", + user: "newuser", + lookupError: errors.New("User not found"), + mockUserID: &user.User{ + Username: "user", + HomeDir: "/home/user", }, initial: []string{ "PATH=/something/else", }, expected: []string{ "PATH=/something/else", - "HOME=/home/www-add", - }, - }, - { - name: "HOME not set and user is set", - user: "newuser", - mockUser: &user.User{ - Username: "newuser", - }, - initial: []string{ - "PATH=/something/else", - }, - expected: []string{ - "PATH=/something/else", - "HOME=/home/newuser", + "HOME=/home/user", }, }, { @@ -122,10 +113,14 @@ func Test_addDefaultHOME(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - userLookup = func(username string) (*user.User, error) { return test.mockUser, nil } - defer func() { userLookup = user.Lookup }() - actual := addDefaultHOME(test.user, test.initial) - testutil.CheckErrorAndDeepEqual(t, false, nil, test.expected, actual) + userLookup = func(username string) (*user.User, error) { return test.mockUser, test.lookupError } + userLookupID = func(username string) (*user.User, error) { return test.mockUserID, nil } + defer func() { + userLookup = user.Lookup + userLookupID = user.LookupId + }() + actual, err := addDefaultHOME(test.user, test.initial) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expected, actual) }) } } diff --git a/pkg/commands/user_test.go b/pkg/commands/user_test.go index ad64d22cd..99775b714 100644 --- a/pkg/commands/user_test.go +++ b/pkg/commands/user_test.go @@ -55,25 +55,25 @@ var userTests = []struct { expectedUID: "fakeUser", }, { - user: "root:root", + user: "root", userObj: &user.User{Uid: "root", Gid: "some"}, - expectedUID: "root:root", + expectedUID: "root", }, { - user: "0:root", + user: "0", userObj: &user.User{Uid: "0"}, - expectedUID: "0:root", + expectedUID: "0", }, { - user: "root:0", + user: "root", userObj: &user.User{Uid: "root"}, - expectedUID: "root:0", + expectedUID: "root", expectedGID: "f0", }, { - user: "0:0", + user: "0", userObj: &user.User{Uid: "0"}, - expectedUID: "0:0", + expectedUID: "0", }, { user: "$envuser", @@ -81,14 +81,14 @@ var userTests = []struct { expectedUID: "root", }, { - user: "root:$envgroup", + user: "root", userObj: &user.User{Uid: "root"}, - expectedUID: "root:grp", + expectedUID: "root", }, { - user: "some:grp", + user: "some", userObj: &user.User{Uid: "some"}, - expectedUID: "some:grp", + expectedUID: "some", }, { user: "some",