separate functions for ownership check to improve readability

Author: michalpristas

internal/pkg/agent/cmd/run_darwin.go

@@ -48,7 +48,7 @@ func checkCapabilitiesPerms(agentCapabilitiesPath string, userName string, uid i
 	} else {
 		capabilitiesUID = os.Getuid()
 	}
-	if err := utils.HasStrictExecPerms(agentCapabilitiesPath, capabilitiesUID, true); err != nil && !os.IsNotExist(err) {
+	if err := utils.HasStrictExecPermsAndOwnership(agentCapabilitiesPath, capabilitiesUID); err != nil && !os.IsNotExist(err) {
 		// capabilities are corrupted, we should not proceed
 		return fmt.Errorf("invalid capabilities file permissions: %w", err)
 	}

internal/pkg/agent/cmd/run_linux.go

@@ -50,7 +50,7 @@ func checkCapabilitiesPerms(agentCapabilitiesPath string, userName string, uid i
 	} else {
 		capabilitiesUID = os.Getuid()
 	}
-	if err := utils.HasStrictExecPerms(agentCapabilitiesPath, capabilitiesUID, true); err != nil && !os.IsNotExist(err) {
+	if err := utils.HasStrictExecPermsAndOwnership(agentCapabilitiesPath, capabilitiesUID); err != nil && !os.IsNotExist(err) {
 		// capabilities are corrupted, we should not proceed
 		return fmt.Errorf("invalid capabilities file permissions: %w", err)
 	}

pkg/component/runtime/command.go

@@ -363,13 +363,12 @@ func (c *commandRuntime) start(comm Communicator) error {
 	}
 	env = append(env, fmt.Sprintf("%s=%s", envAgentComponentID, c.current.ID))
 	env = append(env, fmt.Sprintf("%s=%s", envAgentComponentType, c.getSpecType()))
-	uid := os.Geteuid()
 	workDir := c.current.WorkDirPath(paths.Run())
 	path, err := filepath.Abs(c.getSpecBinaryPath())
 	if err != nil {
 		return fmt.Errorf("failed to determine absolute path: %w", err)
 	}
-	err = utils.HasStrictExecPerms(path, uid, false)
+	err = utils.HasStrictExecPerms(path)
 	if err != nil {
 		return fmt.Errorf("execution of component prevented: %w", err)
 	}

pkg/utils/perm_unix.go

@@ -29,29 +29,49 @@ func CurrentFileOwner() (FileOwner, error) {
 
 // HasStrictExecPerms ensures that the path is executable by the owner, cannot be written by anyone other than the
 // owner of the file and that the owner of the file is the same as the UID or root.
-func HasStrictExecPerms(path string, uid int, checkUID bool) error {
+func HasStrictExecPerms(path string) error {
 	info, err := file.Stat(path)
 	if err != nil {
 		return err
 	}
-	if info.IsDir() {
-		return errors.New("is a directory")
-	}
-	if info.Mode()&0022 != 0 {
-		return errors.New("cannot be writeable by group or other")
+
+	return hasStrictExecPerms(info)
+}
+
+// HasStrictExecPermsAndOwnership ensures that the path is executable by the owner and that the owner of the file
+// is the same as the UID or root.
+func HasStrictExecPermsAndOwnership(path string, uid int) error {
+	info, err := file.Stat(path)
+	if err != nil {
+		return err
 	}
-	if info.Mode()&0100 == 0 {
-		return errors.New("not executable by owner")
+
+	if err := hasStrictExecPerms(info); err != nil {
+		return err
 	}
 
 	fileUID, err := info.UID()
 	if err != nil {
 		return err
 	}
 
-	if checkUID && fileUID != 0 && fileUID != uid {
+	if fileUID != 0 && fileUID != uid {
 		return errors.New("file owner does not match expected UID or root")
 	}
 
 	return nil
 }
+
+func hasStrictExecPerms(info file.FileInfo) error {
+	if info.IsDir() {
+		return errors.New("is a directory")
+	}
+	if info.Mode()&0022 != 0 {
+		return errors.New("cannot be writeable by group or other")
+	}
+	if info.Mode()&0100 == 0 {
+		return errors.New("not executable by owner")
+	}
+
+	return nil
+}

pkg/utils/perm_windows.go

@@ -61,9 +61,15 @@ func CurrentFileOwner() (FileOwner, error) {
 	}, nil
 }
 
-// HasStrictExecPerms ensures that the path is executable by the owner and that the owner of the file
+// HasStrictExecPerms ensures that the path is executable by the owner.
+func HasStrictExecPerms(path string) error {
+	// TODO: Need to add check on Windows to ensure that the ACL are correct for the binary before execution.
+	return nil
+}
+
+// HasStrictExecPermsAndOwnership ensures that the path is executable by the owner and that the owner of the file
 // is the same as the UID or root.
-func HasStrictExecPerms(path string, uid int, _ bool) error {
+func HasStrictExecPermsAndOwnership(path string, uid int) error {
 	// TODO: Need to add check on Windows to ensure that the ACL are correct for the binary before execution.
 	return nil
 }