fails config apply when deleting a file outside allowed dirs, update manifest UT

Author: sean-breen

internal/file/file_manager_service.go

@@ -344,6 +344,12 @@ func (fms *FileManagerService) DetermineFileActions(
 			}
 			fileContents[fileName] = fileContent
 
+			// Allowed directories could have been modified since file was created,
+			// so we should check before marking for deletion
+			if !fms.agentConfig.IsDirectoryAllowed(fileName) {
+				return nil, nil, fmt.Errorf("error deleting file %s: file not in allowed directories", fileName)
+			}
+
 			fileDiff[fileName] = &model.FileCache{
 				File:   manifestFile,
 				Action: model.Delete,

internal/file/file_manager_service_test.go

@@ -462,7 +462,7 @@ func TestFileManagerService_Rollback(t *testing.T) {
 
 func TestFileManagerService_DetermineFileActions(t *testing.T) {
 	ctx := context.Background()
-	tempDir := os.TempDir()
+	tempDir := filepath.Clean(os.TempDir())
 
 	deleteTestFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx_delete.conf")
 	defer helpers.RemoveFileWithErrorCheck(t, deleteTestFile.Name())
@@ -492,6 +492,7 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 	require.NoError(t, addErr)
 
 	tests := []struct {
+		allowedDirs     []string
 		expectedError   error
 		modifiedFiles   map[string]*model.FileCache
 		currentFiles    map[string]*mpi.File
@@ -500,7 +501,8 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 		name            string
 	}{
 		{
-			name: "Test 1: Add, Update & Delete Files",
+			name:        "Test 1: Add, Update & Delete Files",
+			allowedDirs: []string{tempDir},
 			modifiedFiles: map[string]*model.FileCache{
 				addTestFileName: {
 					File: &mpi.File{
@@ -563,7 +565,8 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 			expectedError: nil,
 		},
 		{
-			name: "Test 2: Files same as on disk",
+			name:        "Test 2: Files same as on disk",
+			allowedDirs: []string{tempDir},
 			modifiedFiles: map[string]*model.FileCache{
 				addTestFile.Name(): {
 					File: &mpi.File{
@@ -598,6 +601,7 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 		},
 		{
 			name:          "Test 3: File being deleted already doesn't exist",
+			allowedDirs:   []string{tempDir},
 			modifiedFiles: make(map[string]*model.FileCache),
 			currentFiles: map[string]*mpi.File{
 				"/unknown/file.conf": {
@@ -620,6 +624,7 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 
 			fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
 			fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
+			fileManagerService.agentConfig.AllowedDirectories = test.allowedDirs
 			fileManagerService.agentConfig.LibDir = manifestDirPath
 			fileManagerService.manifestFilePath = manifestFilePath
 
@@ -794,6 +799,7 @@ func TestFileManagerService_UpdateManifestFile(t *testing.T) {
 
 			fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
 			fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
+			fileManagerService.agentConfig.AllowedDirectories = []string{"manifestDirPath"}
 			fileManagerService.agentConfig.LibDir = manifestDirPath
 			fileManagerService.manifestFilePath = file.Name()