worker: Set PATH based on UID instead of container's privileged state

PATH should be set based on a users UID from the perspective of the
container, not what the UID maps to outside the container.

Matches what guardian is doing:
bf4c737f2b/rundmc/processes/unix_env.go (L44-L46)

Now when someone `fly intercept`'s into a container they'll have
`/sbin` on their path and be able to `apt/apk` install whatever debug
tools they need.

Signed-off-by: Taylor Silva <tsilva@pivotal.io>

worker/runtime/container.go

@@ -14,7 +14,12 @@ import (
 	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
-const GraceTimeKey = "garden.grace-time"
+const (
+	SuperuserPath = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+	Path          = "PATH=/usr/local/bin:/usr/bin:/bin"
+
+	GraceTimeKey = "garden.grace-time"
+)
 
 type UserNotFoundError struct {
 	User string
@@ -340,7 +345,6 @@ func (c *Container) setupContainerdProcSpec(gdnProcSpec garden.ProcessSpec, cont
 	procSpec := containerSpec.Process
 
 	procSpec.Args = append([]string{gdnProcSpec.Path}, gdnProcSpec.Args...)
-	procSpec.Env = append(procSpec.Env, gdnProcSpec.Env...)
 
 	cwd := gdnProcSpec.Dir
 	if cwd == "" {
@@ -374,9 +378,19 @@ func (c *Container) setupContainerdProcSpec(gdnProcSpec garden.ProcessSpec, cont
 		setUserEnv := fmt.Sprintf("USER=%s", gdnProcSpec.User)
 		procSpec.Env = append(procSpec.Env, setUserEnv)
 	}
+
+	procSpec.Env = append(procSpec.Env, usersPath(procSpec.User.UID))
 	return *procSpec, nil
 }
 
+func usersPath(uid uint32) string {
+	if uid == 0 {
+		return SuperuserPath
+	}
+
+	return Path
+}
+
 // stdinWrapper will normally transparently pass Reads through to the underlying
 // reader, but if a read fails, it will nop (block) until the process has
 // exited.

worker/runtime/container_test.go

@@ -241,6 +241,66 @@ func (s *ContainerSuite) TestRunWithUserLookupSucceeds() {
 	s.True(userEnvVarSet)
 }
 
+func (s *ContainerSuite) TestRunWithRootUserHasSuperUserPath() {
+	s.containerdContainer.SpecReturns(&specs.Spec{
+		Process: &specs.Process{},
+		Root:    &specs.Root{},
+	}, nil)
+
+	s.containerdContainer.TaskReturns(s.containerdTask, nil)
+	s.containerdTask.ExecReturns(s.containerdProcess, nil)
+
+	expectedUser := specs.User{UID: 0, GID: 0, Username: "root"}
+	s.rootfsManager.LookupUserReturns(expectedUser, true, nil)
+
+	_, err := s.container.Run(garden.ProcessSpec{User: "root"}, garden.ProcessIO{})
+	s.NoError(err)
+
+	_, _, procSpec, _ := s.containerdTask.ExecArgsForCall(0)
+	s.Equal(expectedUser, procSpec.User)
+
+	userEnvVarSet := false
+	expectedEnvVar := runtime.SuperuserPath
+
+	for _, envVar := range procSpec.Env {
+		if envVar == expectedEnvVar {
+			userEnvVarSet = true
+			break
+		}
+	}
+	s.True(userEnvVarSet)
+}
+
+func (s *ContainerSuite) TestRunWithNonRootUserHasUserPath() {
+	s.containerdContainer.SpecReturns(&specs.Spec{
+		Process: &specs.Process{},
+		Root:    &specs.Root{},
+	}, nil)
+
+	s.containerdContainer.TaskReturns(s.containerdTask, nil)
+	s.containerdTask.ExecReturns(s.containerdProcess, nil)
+
+	expectedUser := specs.User{UID: 6, GID: 6, Username: "games"}
+	s.rootfsManager.LookupUserReturns(expectedUser, true, nil)
+
+	_, err := s.container.Run(garden.ProcessSpec{User: "games"}, garden.ProcessIO{})
+	s.NoError(err)
+
+	_, _, procSpec, _ := s.containerdTask.ExecArgsForCall(0)
+	s.Equal(expectedUser, procSpec.User)
+
+	userEnvVarSet := false
+	expectedEnvVar := runtime.Path
+
+	for _, envVar := range procSpec.Env {
+		if envVar == expectedEnvVar {
+			userEnvVarSet = true
+			break
+		}
+	}
+	s.True(userEnvVarSet)
+}
+
 func (s *ContainerSuite) TestRunWithUserLookupErrors() {
 	s.containerdContainer.SpecReturns(&specs.Spec{
 		Process: &specs.Process{},

worker/runtime/spec/spec.go

@@ -11,11 +11,6 @@ import (
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
-const (
-	SuperuserPath = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
-	Path          = "PATH=/usr/local/bin:/usr/bin:/bin"
-)
-
 const baseCgroupsPath = "garden"
 
 var isSwapLimitEnabled bool
@@ -75,8 +70,6 @@ func OciSpec(initBinPath string, gdn garden.ContainerSpec, maxUid, maxGid uint32
 		},
 	)
 
-	oci.Process.Env = envWithDefaultPath(oci.Process.Env, gdn.Privileged)
-
 	return
 }
 
@@ -193,24 +186,6 @@ func OciCgroupsPath(basePath, handle string, privileged bool) string {
 	return filepath.Join(basePath, handle)
 }
 
-// envWithDefaultPath returns the default PATH for a privileged/unprivileged
-// user based on the existence of PATH already being set in the initial
-// environment.
-//
-func envWithDefaultPath(env []string, privileged bool) []string {
-	for _, envVar := range env {
-		if strings.HasPrefix(envVar, "PATH=") {
-			return env
-		}
-	}
-
-	if privileged {
-		return append(env, SuperuserPath)
-	}
-
-	return append(env, Path)
-}
-
 // defaultGardenOciSpec represents a default set of properties necessary in
 // order to satisfy the garden interface.
 //

worker/runtime/spec/spec_test.go

@@ -417,27 +417,6 @@ func (s *SpecSuite) TestContainerSpec() {
 				}
 			},
 		},
-		{
-			desc: "env + default path",
-			gdn: garden.ContainerSpec{
-				Handle: "handle", RootFSPath: "raw:///rootfs",
-				Env: []string{"foo=bar"},
-			},
-			check: func(oci *specs.Spec) {
-				s.Equal([]string{"foo=bar", spec.Path}, oci.Process.Env)
-			},
-		},
-		{
-			desc: "env + default root path",
-			gdn: garden.ContainerSpec{
-				Handle: "handle", RootFSPath: "raw:///rootfs",
-				Env:        []string{"foo=bar"},
-				Privileged: true,
-			},
-			check: func(oci *specs.Spec) {
-				s.Equal([]string{"foo=bar", spec.SuperuserPath}, oci.Process.Env)
-			},
-		},
 		{
 			desc: "env with path already configured",
 			gdn: garden.ContainerSpec{