Do not inherit HOME and USER env vars from pebble daemon environment (#…

…262) Also use root home directory when running tests as sudo
canonical · Aug 8, 2023 · 02ad28a · 02ad28a
1 parent 73aa51e
commit 02ad28a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 5 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -44,12 +44,12 @@ jobs:
     - name: Test
       run: |
         go test -c ./internals/daemon
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E ./daemon.test -check.v -check.f ^execSuite\.TestUserGroup$
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E ./daemon.test -check.v -check.f ^execSuite\.TestUserIDGroupID$
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E ./daemon.test -check.v -check.f ^filesSuite\.TestWriteUserGroupReal$
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E ./daemon.test -check.v -check.f ^filesSuite\.TestMakeDirsUserGroupReal$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserGroup$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^execSuite\.TestUserIDGroupID$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestWriteUserGroupReal$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestMakeDirsUserGroupReal$
         go test -c ./internals/overlord/servstate/
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E ./servstate.test -check.v -check.f ^S.TestUserGroup$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./servstate.test -check.v -check.f ^S.TestUserGroup$
 
   format:
     runs-on: ubuntu-latest

diff --git a/internals/daemon/api_files_test.go b/internals/daemon/api_files_test.go
@@ -70,6 +70,9 @@ func (s *filesSuite) TestListFilesNonAbsPath(c *C) {
 }
 
 func (s *filesSuite) TestListFilesPermissionDenied(c *C) {
+	if os.Getuid() == 0 {
+		c.Skip("cannot run test as root")
+	}
 	tmpDir := c.MkDir()
 	noAccessDir := filepath.Join(tmpDir, "noaccess")
 	c.Assert(os.Mkdir(noAccessDir, 0o775), IsNil)
@@ -302,6 +305,10 @@ func (s *filesSuite) TestReadMultiple(c *C) {
 }
 
 func (s *filesSuite) TestReadErrors(c *C) {
+	if os.Getuid() == 0 {
+		c.Skip("cannot run test as root")
+	}
+
 	tmpDir := createTestFiles(c)
 	writeTempFile(c, tmpDir, "no-access", "x", 0)
 
@@ -1122,6 +1129,10 @@ nested user group
 }
 
 func (s *filesSuite) TestWriteErrors(c *C) {
+	if os.Getuid() == 0 {
+		c.Skip("cannot run test as root")
+	}
+
 	tmpDir := c.MkDir()
 	c.Assert(os.Mkdir(tmpDir+"/permission-denied", 0), IsNil)
 	pathNoContent := tmpDir + "/no-content"

diff --git a/internals/overlord/cmdstate/request.go b/internals/overlord/cmdstate/request.go
@@ -71,7 +71,14 @@ func Exec(st *state.State, args *ExecArgs) (*state.Task, ExecMetadata, error) {
 	}
 
 	// Inherit the pebble daemon environment.
+	// If the user is being changed, unset the HOME and USER env vars so that they
+	// can be set correctly later on in this method.
 	environment := osutil.Environ()
+	if args.UserID != nil && *args.UserID != os.Getuid() {
+		delete(environment, "HOME")
+		delete(environment, "USER")
+	}
+
 	for k, v := range args.Environment {
 		// Requested environment takes precedence.
 		environment[k] = v