Cherry-pick MM-66789 (Include log viewer (system console) in log root path validation) (#35253)

Automatic Merge

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)

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)