From a8db85c02634c485cdd655b564c66a3e746e59fa Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Fri, 13 Feb 2026 05:09:37 -0500 Subject: [PATCH] Cherry-pick MM-66789 (Include log viewer (system console) in log root path validation) (#35253) Automatic Merge --- server/channels/app/platform/log.go | 12 ++++- server/channels/app/platform/log_test.go | 49 +++++++++++++++++++ .../admin_console/server_logs/log_list.tsx | 2 +- webapp/channels/src/i18n/en.json | 2 +- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/server/channels/app/platform/log.go b/server/channels/app/platform/log.go index b2c9788a4a..cb20ddc346 100644 --- a/server/channels/app/platform/log.go +++ b/server/channels/app/platform/log.go @@ -134,6 +134,16 @@ func (ps *PlatformService) GetLogsSkipSend(rctx request.CTX, page, perPage int, if *ps.Config().LogSettings.EnableFile { ps.Log().Flush() logFile := config.GetLogFileLocation(*ps.Config().LogSettings.FileLocation) + + // Validate the file path to prevent arbitrary file reads + if err := ps.validateLogFilePath(logFile); err != nil { + rctx.Logger().Error("Blocked attempt to read log file outside allowed root", + mlog.String("path", logFile), + mlog.String("config_section", "LogSettings.FileLocation"), + mlog.Err(err)) + return nil, model.NewAppError("getLogs", "api.admin.file_read_error", nil, "", http.StatusForbidden).Wrap(err) + } + file, err := os.Open(logFile) if err != nil { return nil, model.NewAppError("getLogs", "api.admin.file_read_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -272,7 +282,7 @@ func (ps *PlatformService) GetNotificationLogFile(rctx request.CTX) (*model.File // validateLogFilePath validates that a log file path is within the logging root directory. // This prevents arbitrary file read/write vulnerabilities in logging configuration. // The logging root is determined by MM_LOG_PATH environment variable or the default logs directory. -// Currently used to validate paths when reading logs via GetAdvancedLogs. +// Used to validate paths when reading logs via GetLogsSkipSend, GetLogFile, and GetAdvancedLogs. // In future versions, this will also be used to validate paths when saving logging config. func (ps *PlatformService) validateLogFilePath(filePath string) error { // Get the logging root path (from env var or default logs directory) diff --git a/server/channels/app/platform/log_test.go b/server/channels/app/platform/log_test.go index 771d63c368..bd80e7e566 100644 --- a/server/channels/app/platform/log_test.go +++ b/server/channels/app/platform/log_test.go @@ -187,6 +187,55 @@ func TestGetNotificationLogFile(t *testing.T) { }) } +func TestGetLogsSkipSendPathValidation(t *testing.T) { + mainHelper.Parallel(t) + + th := Setup(t) + defer th.TearDown() + + t.Run("path validation prevents reading files outside log directory", func(t *testing.T) { + // Create a directory to use as the allowed log root + logDir, err := os.MkdirTemp("", "logs") + require.NoError(t, err) + t.Cleanup(func() { + th.Service.UpdateConfig(func(cfg *model.Config) { + *cfg.LogSettings.EnableFile = false + }) + th.Service.Logger().Flush() + err = os.RemoveAll(logDir) + require.NoError(t, err) + }) + + // Set MM_LOG_PATH to restrict log file access to logDir + t.Setenv("MM_LOG_PATH", logDir) + + // Create a directory outside the allowed log root + outsideDir, err := os.MkdirTemp("", "outside") + require.NoError(t, err) + t.Cleanup(func() { + err = os.RemoveAll(outsideDir) + require.NoError(t, err) + }) + + // Create a log file outside the allowed root that should not be readable + outsideLogLocation := config.GetLogFileLocation(outsideDir) + err = os.WriteFile(outsideLogLocation, []byte("secret data\n"), 0644) + require.NoError(t, err) + + // Point FileLocation to the outside directory + th.Service.UpdateConfig(func(cfg *model.Config) { + *cfg.LogSettings.EnableFile = true + *cfg.LogSettings.FileLocation = outsideDir + }) + + // Should be blocked by path validation + lines, appErr := th.Service.GetLogsSkipSend(th.Context, 0, 10, &model.LogFilter{}) + assert.Nil(t, lines) + require.NotNil(t, appErr) + assert.Equal(t, "api.admin.file_read_error", appErr.Id) + }) +} + func TestGetAdvancedLogs(t *testing.T) { mainHelper.Parallel(t) diff --git a/webapp/channels/src/components/admin_console/server_logs/log_list.tsx b/webapp/channels/src/components/admin_console/server_logs/log_list.tsx index 957910e80b..956e7294d6 100644 --- a/webapp/channels/src/components/admin_console/server_logs/log_list.tsx +++ b/webapp/channels/src/components/admin_console/server_logs/log_list.tsx @@ -272,7 +272,7 @@ export default class LogList extends React.PureComponent { const placeholderEmpty: JSX.Element = ( ); diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index a739e02b44..f2d89434c1 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -601,7 +601,7 @@ "admin.channel_settings.channel_list.managementHeader": "Management", "admin.channel_settings.channel_list.nameHeader": "Name", "admin.channel_settings.channel_list.no_channels_found": "No channels found", - "admin.channel_settings.channel_list.no_logs_found": "No logs found", + "admin.channel_settings.channel_list.no_logs_found": "No logs found. Ensure log files are within the logging root directory (configured via MM_LOG_PATH or the default logs directory).", "admin.channel_settings.channel_list.search_channels_errored": "Something went wrong. Try again", "admin.channel_settings.channel_list.teamHeader": "Team", "admin.channel_settings.channel_moderation.channelMentions": "Channel Mentions",