Skip to content

Commit 5642dbe

Browse files
authored
Fix file permission validation (#1316)
* add file permission validation * add unit test for config apply with file with execute permissions
1 parent 3df2793 commit 5642dbe

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

internal/file/file_manager_service.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
167167
return model.Error, allowedErr
168168
}
169169

170+
permissionErr := fms.validateAndUpdateFilePermissions(ctx, fileOverview.GetFiles())
171+
if permissionErr != nil {
172+
return model.RollbackRequired, permissionErr
173+
}
174+
170175
diffFiles, compareErr := fms.DetermineFileActions(
171176
ctx,
172177
fms.currentFilesOnDisk,

internal/file/file_manager_service_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,54 @@ func TestFileManagerService_ConfigApply_Failed(t *testing.T) {
301301
assert.False(t, fileManagerService.rollbackManifest)
302302
}
303303

304+
func TestFileManagerService_ConfigApply_FileWithExecutePermissions(t *testing.T) {
305+
ctx := context.Background()
306+
tempDir := t.TempDir()
307+
308+
filePath := filepath.Join(tempDir, "nginx.conf")
309+
310+
fileContent := []byte("location /test {\n return 200 \"Test location\\n\";\n}")
311+
fileHash := files.GenerateHash(fileContent)
312+
defer helpers.RemoveFileWithErrorCheck(t, filePath)
313+
314+
overview := protos.FileOverview(filePath, fileHash)
315+
316+
overview.GetFiles()[0].GetFileMeta().Permissions = "0755"
317+
318+
manifestDirPath := tempDir
319+
manifestFilePath := filepath.Join(manifestDirPath, "manifest.json")
320+
helpers.CreateFileWithErrorCheck(t, manifestDirPath, "manifest.json")
321+
322+
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
323+
fakeFileServiceClient.GetOverviewReturns(&mpi.GetOverviewResponse{
324+
Overview: overview,
325+
}, nil)
326+
fakeFileServiceClient.GetFileReturns(&mpi.GetFileResponse{
327+
Contents: &mpi.FileContents{
328+
Contents: fileContent,
329+
},
330+
}, nil)
331+
agentConfig := types.AgentConfig()
332+
agentConfig.AllowedDirectories = []string{tempDir}
333+
334+
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig, &sync.RWMutex{})
335+
fileManagerService.configPath = filepath.Dir(filePath)
336+
fileManagerService.agentConfig.LibDir = manifestDirPath
337+
fileManagerService.manifestFilePath = manifestFilePath
338+
339+
request := protos.CreateConfigApplyRequest(overview)
340+
writeStatus, err := fileManagerService.ConfigApply(ctx, request)
341+
require.NoError(t, err)
342+
assert.Equal(t, model.OK, writeStatus)
343+
assert.Equal(t, "0644", fileManagerService.fileActions[filePath].File.GetFileMeta().GetPermissions())
344+
data, readErr := os.ReadFile(filePath)
345+
require.NoError(t, readErr)
346+
assert.Equal(t, fileContent, data)
347+
assert.Equal(t, fileManagerService.fileActions[filePath].File, overview.GetFiles()[0])
348+
assert.Equal(t, 1, fakeFileServiceClient.GetFileCallCount())
349+
assert.True(t, fileManagerService.rollbackManifest)
350+
}
351+
304352
func TestFileManagerService_checkAllowedDirectory(t *testing.T) {
305353
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
306354
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})

0 commit comments

Comments
 (0)