parse user's permissions and remove execute bit

Author: spencerugbo

internal/file/file_manager_service.go

@@ -38,6 +38,7 @@ const (
 	maxAttempts = 5
 	dirPerm     = 0o755
 	filePerm    = 0o600
+	executePerm = 0o111
 )
 
 type (
@@ -627,26 +628,34 @@ func (fms *FileManagerService) validateAndFixPermissions(ctx context.Context, fi
 func (fms *FileManagerService) checkFilePermissions(file *mpi.File) error {
 	filePermission := file.GetFileMeta().GetPermissions()
 
-	permissionCodes := filePermission[1:]
-
-	for _, digit := range permissionCodes {
-		singleCode := digit - '0'
+	permissionOctal, err := strconv.ParseUint(filePermission, 8, 32)
+	if err != nil {
+		return fmt.Errorf("file %s has malformed permissions", file.GetFileMeta().GetName())
+	}
 
-		if singleCode&1 != 0 {
-			return fmt.Errorf("file %s has execute permissions", file.GetFileMeta().GetName())
-		}
+	if permissionOctal&executePerm > 0 {
+		return fmt.Errorf("file %s has execute permissions", file.GetFileMeta().GetName())
 	}
 
 	return nil
 }
 
 func (fms *FileManagerService) resetFilePermissions(file *mpi.File) error {
-	perm, err := strconv.ParseUint("0644", 8, 32)
+	filePermission := file.GetFileMeta().GetPermissions()
+
+	permissionOctal, err := strconv.ParseUint(filePermission, 8, 32)
 	if err != nil {
-		return fmt.Errorf("error parsing file permissions: %w", err)
+		chmodErr := os.Chmod(file.GetFileMeta().GetName(), os.FileMode(filePerm))
+		if chmodErr != nil {
+			return fmt.Errorf("failed to set file permissions: %w", err)
+		}
+
+		return fmt.Errorf("falied to parse file permissions, permissions set to 0o644: %w", err)
 	}
 
-	err = os.Chmod(file.GetFileMeta().GetName(), os.FileMode(perm))
+	newPermission := permissionOctal &^ executePerm
+
+	err = os.Chmod(file.GetFileMeta().GetName(), os.FileMode(newPermission))
 	if err != nil {
 		return fmt.Errorf("failed to set file permissions: %w", err)
 	}

internal/file/file_manager_service_test.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strconv"
 	"sync"
 	"testing"
 
@@ -335,7 +336,7 @@ func TestFileManagerService_validateAndFixPermissions(t *testing.T) {
 
 	err := os.Chmod(execFile.Name(), 0o700)
 	require.NoError(t, err)
-	err = os.Chmod(normalFile.Name(), 0o600)
+	err = os.Chmod(normalFile.Name(), 0o620)
 	require.NoError(t, err)
 
 	fileList := []*mpi.File{
@@ -348,7 +349,7 @@ func TestFileManagerService_validateAndFixPermissions(t *testing.T) {
 		{
 			FileMeta: &mpi.FileMeta{
 				Name:        normalFile.Name(),
-				Permissions: "0600",
+				Permissions: "0620",
 			},
 		},
 	}
@@ -361,11 +362,11 @@ func TestFileManagerService_validateAndFixPermissions(t *testing.T) {
 
 	info, err := os.Stat(execFile.Name())
 	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0o644), info.Mode().Perm())
+	assert.Equal(t, os.FileMode(0o600), info.Mode().Perm())
 
 	info, err = os.Stat(normalFile.Name())
 	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0o600), info.Mode().Perm())
+	assert.Equal(t, os.FileMode(0o620), info.Mode().Perm())
 }
 
 func TestFileManagerService_checkFilePermissions(t *testing.T) {
@@ -382,22 +383,17 @@ func TestFileManagerService_checkFilePermissions(t *testing.T) {
 			permissions: "0600",
 			expectError: false,
 		},
-		{
-			name:        "File with read permissions for all",
-			permissions: "0444",
-			expectError: false,
-		},
 		{
 			name:        "File with read/write and execute permissions for owner",
 			permissions: "0700",
 			expectError: true,
 			errorMsg:    "has execute permissions",
 		},
 		{
-			name:        "File with execute permission for all",
-			permissions: "0777",
+			name:        "File with malformed permissions",
+			permissions: "abcde",
 			expectError: true,
-			errorMsg:    "has execute permissions",
+			errorMsg:    "has malformed permissions",
 		},
 	}
 
@@ -424,26 +420,59 @@ func TestFileManagerService_checkFilePermissions(t *testing.T) {
 
 func TestFileManagerService_resetFilePermissions(t *testing.T) {
 	fileManagerService := NewFileManagerService(nil, types.AgentConfig(), &sync.RWMutex{})
-
 	tempDir := t.TempDir()
-	tempFile := helpers.CreateFileWithErrorCheck(t, tempDir, "test.conf")
-	defer helpers.RemoveFileWithErrorCheck(t, tempFile.Name())
 
-	err := os.Chmod(tempFile.Name(), 0o700)
-	require.NoError(t, err)
-
-	file := &mpi.File{
-		FileMeta: &mpi.FileMeta{
-			Name: tempFile.Name(),
+	tests := []struct {
+		name              string
+		permissions       string
+		errorMsg          string
+		expectPermissions string
+		expectError       bool
+	}{
+		{
+			name:              "File with execute permissions for owner and others",
+			permissions:       "0703",
+			expectError:       false,
+			expectPermissions: "0602",
+		},
+		{
+			name:              "File with malformed permissions",
+			permissions:       "abcde",
+			expectError:       true,
+			expectPermissions: "0600",
+			errorMsg:          "falied to parse file permissions",
 		},
 	}
 
-	err = fileManagerService.resetFilePermissions(file)
-	require.NoError(t, err)
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			tempFile := helpers.CreateFileWithErrorCheck(t, tempDir, "test.conf")
+			defer helpers.RemoveFileWithErrorCheck(t, tempFile.Name())
 
-	info, err := os.Stat(tempFile.Name())
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0o644), info.Mode().Perm())
+			file := &mpi.File{
+				FileMeta: &mpi.FileMeta{
+					Name:        tempFile.Name(),
+					Permissions: test.permissions,
+				},
+			}
+
+			parseErr := fileManagerService.resetFilePermissions(file)
+
+			info, err := os.Stat(tempFile.Name())
+			require.NoError(t, err)
+
+			actual := "0" + strconv.FormatUint(uint64(info.Mode().Perm()), 8)
+
+			if test.expectError {
+				require.Error(t, parseErr)
+				assert.Equal(t, test.expectPermissions, actual)
+				assert.Contains(t, parseErr.Error(), test.errorMsg)
+			} else {
+				require.NoError(t, parseErr)
+				assert.Equal(t, test.expectPermissions, actual)
+			}
+		})
+	}
 }
 
 func TestFileManagerService_ClearCache(t *testing.T) {