feat(osutil.Mkdir): add the chmod flag (#423)

Add a new `Chmod` flag to `MkdirOptions` so that an explicit `chmod` command is executed on all directories created by `osutil.Mkdir`.

.github/workflows/tests.yml

@@ -49,7 +49,7 @@ jobs:
         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/osutil
-        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./osutil.test -check.v -check.f ^mkdirSuite\.TestMakeParentsAndChown$
+        PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./osutil.test -check.v -check.f ^mkdirSuite\.TestMakeParentsChmodAndChown$
         
         go test -c ./internals/overlord/servstate/
         PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./servstate.test -check.v -check.f ^S.TestUserGroup$

internals/daemon/api_files.go

@@ -504,6 +504,7 @@ func mkdirAllUserGroup(path string, perm os.FileMode, uid, gid *int) error {
 		return mkdir(path, perm, &osutil.MkdirOptions{
 			MakeParents: true,
 			ExistOK:     true,
+			Chmod:       true,
 			Chown:       true,
 			UserID:      sys.UserID(*uid),
 			GroupID:     sys.GroupID(*gid),
@@ -512,19 +513,23 @@ func mkdirAllUserGroup(path string, perm os.FileMode, uid, gid *int) error {
 		return mkdir(path, perm, &osutil.MkdirOptions{
 			MakeParents: true,
 			ExistOK:     true,
+			Chmod:       true,
 		})
 	}
 }
 
 func mkdirUserGroup(path string, perm os.FileMode, uid, gid *int) error {
 	if uid != nil && gid != nil {
 		return mkdir(path, perm, &osutil.MkdirOptions{
+			Chmod:   true,
 			Chown:   true,
 			UserID:  sys.UserID(*uid),
 			GroupID: sys.GroupID(*gid),
 		})
 	} else {
-		return mkdir(path, perm, nil)
+		return mkdir(path, perm, &osutil.MkdirOptions{
+			Chmod: true,
+		})
 	}
 }
 

internals/daemon/api_files_test.go

@@ -480,11 +480,11 @@ func (s *filesSuite) TestMakeDirsUserGroupMocked(c *C) {
 	tmpDir := s.testMakeDirsUserGroup(c, 12, 34, "USER", "GROUP")
 
 	c.Assert(mkdirCalls, HasLen, 5)
-	c.Check(mkdirCalls[0], Equals, args{tmpDir + "/normal", 0o755, osutil.MkdirOptions{}})
-	c.Check(mkdirCalls[1], Equals, args{tmpDir + "/uid-gid", 0o755, osutil.MkdirOptions{Chown: true, UserID: 12, GroupID: 34}})
-	c.Check(mkdirCalls[2], Equals, args{tmpDir + "/user-group", 0o755, osutil.MkdirOptions{Chown: true, UserID: 56, GroupID: 78}})
-	c.Check(mkdirCalls[3], Equals, args{tmpDir + "/nested1/normal", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true}})
-	c.Check(mkdirCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chown: true, UserID: 56, GroupID: 78}})
+	c.Check(mkdirCalls[0], Equals, args{tmpDir + "/normal", 0o755, osutil.MkdirOptions{Chmod: true}})
+	c.Check(mkdirCalls[1], Equals, args{tmpDir + "/uid-gid", 0o755, osutil.MkdirOptions{Chmod: true, Chown: true, UserID: 12, GroupID: 34}})
+	c.Check(mkdirCalls[2], Equals, args{tmpDir + "/user-group", 0o755, osutil.MkdirOptions{Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
+	c.Check(mkdirCalls[3], Equals, args{tmpDir + "/nested1/normal", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true}})
+	c.Check(mkdirCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
 }
 
 func (s *filesSuite) testMakeDirsUserGroup(c *C, uid, gid int, user, group string) string {
@@ -984,8 +984,8 @@ func (s *filesSuite) TestWriteUserGroupMocked(c *C) {
 	c.Check(atomicWriteChownCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o644, 56, 78})
 
 	c.Assert(mkdirCalls, HasLen, 2)
-	c.Check(mkdirCalls[0], Equals, mkdirArgs{tmpDir + "/nested1", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true}})
-	c.Check(mkdirCalls[1], Equals, mkdirArgs{tmpDir + "/nested2", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chown: true, UserID: 56, GroupID: 78}})
+	c.Check(mkdirCalls[0], Equals, mkdirArgs{tmpDir + "/nested1", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true}})
+	c.Check(mkdirCalls[1], Equals, mkdirArgs{tmpDir + "/nested2", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
 }
 
 // See .github/workflows/tests.yml for how to run this test as root.

internals/osutil/mkdir.go

@@ -41,6 +41,12 @@ type MkdirOptions struct {
 	// it's not a directory).
 	ExistOK bool
 
+	// If false (the default), no explicit chmod is performed. In this case, the permission
+	// of the created directories will be affected by umask settings.
+	//
+	// If true, perform an explicit chmod on any directories created.
+	Chmod bool
+
 	// If false (the default), no explicit chown is performed.
 	// If true, perform an explicit chown on any directories created, using the UserID
 	// and GroupID provided.
@@ -119,7 +125,7 @@ func mkdirAll(path string, perm os.FileMode, options *MkdirOptions) error {
 	return mkdir(path, perm, options)
 }
 
-// Create a single directory and perform chown operations according to options.
+// Create a single directory and perform chown/chmod operations according to options.
 func mkdir(path string, perm os.FileMode, options *MkdirOptions) error {
 	cand := path + ".mkdir-new"
 
@@ -133,6 +139,12 @@ func mkdir(path string, perm os.FileMode, options *MkdirOptions) error {
 		}
 	}
 
+	if options.Chmod {
+		if err := os.Chmod(cand, perm); err != nil {
+			return err
+		}
+	}
+
 	if err := os.Rename(cand, path); err != nil {
 		return err
 	}

internals/osutil/mkdir_test.go

@@ -30,7 +30,7 @@ type mkdirSuite struct{}
 
 var _ = check.Suite(&mkdirSuite{})
 
-// Chown requires root, so it's not tested, only test MakeParents, ExistOK,
+// Chown requires root, so it's not tested, only test MakeParents, ExistOK, Chmod,
 // and the combination of them.
 func (mkdirSuite) TestSimpleDir(c *check.C) {
 	tmpDir := c.MkDir()
@@ -112,8 +112,82 @@ func (mkdirSuite) TestMakeParentsAndExistNotOK(c *check.C) {
 	c.Assert(err, check.ErrorMatches, `.*: file exists`)
 }
 
+func (mkdirSuite) TestChmod(c *check.C) {
+	oldmask := syscall.Umask(0022)
+	defer syscall.Umask(oldmask)
+
+	tmpDir := c.MkDir()
+
+	err := osutil.Mkdir(tmpDir+"/foo", 0o777, &osutil.MkdirOptions{
+		Chmod: true,
+	})
+	c.Assert(err, check.IsNil)
+	c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
+
+	info, err := os.Stat(tmpDir + "/foo")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
+}
+
+func (mkdirSuite) TestNoChmod(c *check.C) {
+	oldmask := syscall.Umask(0022)
+	defer syscall.Umask(oldmask)
+
+	tmpDir := c.MkDir()
+
+	err := osutil.Mkdir(tmpDir+"/foo", 0o777, nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
+
+	info, err := os.Stat(tmpDir + "/foo")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))
+}
+
+func (mkdirSuite) TestMakeParentsAndChmod(c *check.C) {
+	tmpDir := c.MkDir()
+
+	err := osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
+		MakeParents: true,
+		Chmod:       true,
+	})
+	c.Assert(err, check.IsNil)
+	c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
+	c.Assert(osutil.IsDir(tmpDir+"/foo/bar"), check.Equals, true)
+
+	info, err := os.Stat(tmpDir + "/foo")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
+
+	info, err = os.Stat(tmpDir + "/foo/bar")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
+}
+
+func (mkdirSuite) TestMakeParentsAndNoChmod(c *check.C) {
+	oldmask := syscall.Umask(0022)
+	defer syscall.Umask(oldmask)
+
+	tmpDir := c.MkDir()
+
+	err := osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
+		MakeParents: true,
+	})
+	c.Assert(err, check.IsNil)
+	c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
+	c.Assert(osutil.IsDir(tmpDir+"/foo/bar"), check.Equals, true)
+
+	info, err := os.Stat(tmpDir + "/foo")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))
+
+	info, err = os.Stat(tmpDir + "/foo/bar")
+	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))
+}
+
 // See .github/workflows/tests.yml for how to run this test as root.
-func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {
+func (mkdirSuite) TestMakeParentsChmodAndChown(c *check.C) {
 	if os.Getuid() != 0 {
 		c.Skip("requires running as root")
 	}
@@ -136,6 +210,7 @@ func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {
 
 	err = osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
 		MakeParents: true,
+		Chmod:       true,
 		Chown:       true,
 		UserID:      sys.UserID(uid),
 		GroupID:     sys.GroupID(gid),
@@ -146,13 +221,15 @@ func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {
 
 	info, err := os.Stat(tmpDir + "/foo")
 	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
 	stat, ok := info.Sys().(*syscall.Stat_t)
 	c.Assert(ok, check.Equals, true)
 	c.Assert(int(stat.Uid), check.Equals, uid)
 	c.Assert(int(stat.Gid), check.Equals, gid)
 
 	info, err = os.Stat(tmpDir + "/foo/bar")
 	c.Assert(err, check.IsNil)
+	c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
 	stat, ok = info.Sys().(*syscall.Stat_t)
 	c.Assert(ok, check.Equals, true)
 	c.Assert(int(stat.Uid), check.Equals, uid)