Cherry-pick MM-66789: Restrict log downloads to a root path for support packets (#35164)

Automatic Merge

server/channels/api4/apitestlib.go

@@ -106,7 +106,14 @@ func setupTestHelper(tb testing.TB, dbStore store.Store, sqlSettings *model.SqlS
 		consoleLevel = mlog.LvlStdLog.Name
 	}
 	*memoryConfig.LogSettings.ConsoleLevel = consoleLevel
-	*memoryConfig.LogSettings.FileLocation = filepath.Join(tempWorkspace, "logs", "mattermost.log")
+	// Use a subdirectory within the logging root (from MM_LOG_PATH or default)
+	// to ensure the path is within the allowed logging root for security validation.
+	// Each test gets its own subdirectory based on the tempWorkspace name for isolation.
+	testLogsDir := filepath.Join(config.GetLogRootPath(), filepath.Base(tempWorkspace))
+	err = os.MkdirAll(testLogsDir, 0700)
+	require.NoError(tb, err, "failed to create test logs directory")
+	*memoryConfig.LogSettings.FileLocation = testLogsDir
+	*memoryConfig.NotificationLogSettings.FileLocation = testLogsDir
 	*memoryConfig.AnnouncementSettings.AdminNoticesEnabled = false
 	*memoryConfig.AnnouncementSettings.UserNoticesEnabled = false
 	*memoryConfig.PluginSettings.AutomaticPrepackagedPlugins = false

server/channels/api4/system.go

@@ -88,6 +88,9 @@ func generateSupportPacket(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	auditRec := c.MakeAuditRecord(model.AuditEventGenerateSupportPacket, model.AuditStatusFail)
+	defer c.LogAuditRec(auditRec)
+
 	// We support the existing API hence the logs are always included
 	// if nothing specified.
 	includeLogs := true
@@ -99,6 +102,9 @@ func generateSupportPacket(c *Context, w http.ResponseWriter, r *http.Request) {
 		PluginPackets: r.Form["plugin_packets"],
 	}
 
+	auditRec.AddMeta("include_logs", supportPacketOptions.IncludeLogs)
+	auditRec.AddMeta("plugin_packets", supportPacketOptions.PluginPackets)
+
 	// Checking to see if the server has a e10 or e20 license (this feature is only permitted for servers with licenses)
 	if c.App.Channels().License() == nil {
 		c.Err = model.NewAppError("Api4.generateSupportPacket", "api.no_license", nil, "", http.StatusForbidden)
@@ -132,6 +138,9 @@ func generateSupportPacket(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 	fileBytesReader := bytes.NewReader(fileBytes)
 
+	auditRec.Success()
+	auditRec.AddMeta("filename", outputZipFilename)
+
 	// Prevent caching so support packets are always fresh
 	w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
 

server/channels/app/helper_test.go

@@ -78,7 +78,13 @@ func setupTestHelper(dbStore store.Store, sqlStore *sqlstore.SqlStore, sqlSettin
 	*memoryConfig.LogSettings.ConsoleLevel = mlog.LvlStdLog.Name
 	*memoryConfig.AnnouncementSettings.AdminNoticesEnabled = false
 	*memoryConfig.AnnouncementSettings.UserNoticesEnabled = false
-	*memoryConfig.LogSettings.FileLocation = filepath.Join(tempWorkspace, "logs", "mattermost.log")
+	// Use a subdirectory within the logging root (from MM_LOG_PATH or default)
+	// to ensure the path is within the allowed logging root for security validation.
+	// Each test gets its own subdirectory based on the tempWorkspace name for isolation.
+	testLogsDir := filepath.Join(config.GetLogRootPath(), filepath.Base(tempWorkspace))
+	err = os.MkdirAll(testLogsDir, 0700)
+	require.NoError(tb, err, "failed to create test logs directory")
+	*memoryConfig.LogSettings.FileLocation = testLogsDir
 	if updateConfig != nil {
 		updateConfig(memoryConfig)
 	}

server/channels/app/platform/config.go

@@ -107,6 +107,9 @@ func (ps *PlatformService) SaveConfig(newCfg *model.Config, sendConfigChangeClus
 		}
 	}
 
+	// Validate log file paths (logs errors for now, will block server startup in future version)
+	config.WarnIfLogPathsOutsideRoot(newCfg)
+
 	oldCfg, newCfg, err := ps.configStore.Set(newCfg)
 	if errors.Is(err, config.ErrReadOnlyConfiguration) {
 		return nil, nil, model.NewAppError("saveConfig", "ent.cluster.save_config.error", nil, "", http.StatusForbidden).Wrap(err)

server/channels/app/platform/log.go

@@ -215,12 +215,22 @@ func (ps *PlatformService) GetLogsSkipSend(rctx request.CTX, page, perPage int,
 	return lines, nil
 }
 
-func (ps *PlatformService) GetLogFile(_ request.CTX) (*model.FileData, error) {
+func (ps *PlatformService) GetLogFile(rctx request.CTX) (*model.FileData, error) {
 	if !*ps.Config().LogSettings.EnableFile {
 		return nil, errors.New("Unable to retrieve mattermost logs because LogSettings.EnableFile is set to false")
 	}
 
 	mattermostLog := config.GetLogFileLocation(*ps.Config().LogSettings.FileLocation)
+
+	// Validate the file path to prevent arbitrary file reads
+	if err := ps.validateLogFilePath(mattermostLog); err != nil {
+		rctx.Logger().Error("Blocked attempt to read log file outside allowed root",
+			mlog.String("path", mattermostLog),
+			mlog.String("config_section", "LogSettings.FileLocation"),
+			mlog.Err(err))
+		return nil, errors.Wrapf(err, "log file path %s is outside allowed logging directory", mattermostLog)
+	}
+
 	mattermostLogFileData, err := os.ReadFile(mattermostLog)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed read mattermost log file at path %s", mattermostLog)
@@ -232,12 +242,22 @@ func (ps *PlatformService) GetLogFile(_ request.CTX) (*model.FileData, error) {
 	}, nil
 }
 
-func (ps *PlatformService) GetNotificationLogFile(_ request.CTX) (*model.FileData, error) {
+func (ps *PlatformService) GetNotificationLogFile(rctx request.CTX) (*model.FileData, error) {
 	if !*ps.Config().NotificationLogSettings.EnableFile {
 		return nil, errors.New("Unable to retrieve notifications logs because NotificationLogSettings.EnableFile is set to false")
 	}
 
 	notificationsLog := config.GetNotificationsLogFileLocation(*ps.Config().NotificationLogSettings.FileLocation)
+
+	// Validate the file path to prevent arbitrary file reads
+	if err := ps.validateLogFilePath(notificationsLog); err != nil {
+		rctx.Logger().Error("Blocked attempt to read log file outside allowed root",
+			mlog.String("path", notificationsLog),
+			mlog.String("config_section", "NotificationLogSettings.FileLocation"),
+			mlog.Err(err))
+		return nil, errors.Wrapf(err, "log file path %s is outside allowed logging directory", notificationsLog)
+	}
+
 	notificationsLogFileData, err := os.ReadFile(notificationsLog)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed read notifcation log file at path %s", notificationsLog)
@@ -249,12 +269,26 @@ func (ps *PlatformService) GetNotificationLogFile(_ request.CTX) (*model.FileDat
 	}, nil
 }
 
-func (ps *PlatformService) GetAdvancedLogs(_ request.CTX) ([]*model.FileData, error) {
+// 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.
+// 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)
+	loggingRoot := config.GetLogRootPath()
+
+	return config.ValidateLogFilePath(filePath, loggingRoot)
+}
+
+func (ps *PlatformService) GetAdvancedLogs(rctx request.CTX) ([]*model.FileData, error) {
 	var (
 		rErr *multierror.Error
 		ret  []*model.FileData
 	)
 
+	rctx.Logger().Debug("Advanced logs access requested")
+
 	for name, loggingJSON := range map[string]json.RawMessage{
 		"LogSettings.AdvancedLoggingJSON":               ps.Config().LogSettings.AdvancedLoggingJSON,
 		"NotificationLogSettings.AdvancedLoggingJSON":   ps.Config().NotificationLogSettings.AdvancedLoggingJSON,
@@ -282,6 +316,18 @@ func (ps *PlatformService) GetAdvancedLogs(_ request.CTX) ([]*model.FileData, er
 				rErr = multierror.Append(rErr, errors.Wrapf(err, "error decoding file target options in %s", name))
 				continue
 			}
+
+			// Validate the file path to prevent arbitrary file reads
+			if err := ps.validateLogFilePath(fileOption.Filename); err != nil {
+				rctx.Logger().Error("Blocked attempt to read log file outside allowed root",
+					mlog.String("path", fileOption.Filename),
+					mlog.String("config_section", name),
+					mlog.String("user_id", rctx.Session().UserId),
+					mlog.Err(err))
+				rErr = multierror.Append(rErr, errors.Wrapf(err, "log file path %s in %s is outside allowed logging directory", fileOption.Filename, name))
+				continue
+			}
+
 			data, err := os.ReadFile(fileOption.Filename)
 			if err != nil {
 				rErr = multierror.Append(rErr, errors.Wrapf(err, "failed to read advanced log file at path %s in %s", fileOption.Filename, name))
@@ -296,7 +342,7 @@ func (ps *PlatformService) GetAdvancedLogs(_ request.CTX) ([]*model.FileData, er
 		}
 	}
 
-	return ret, nil
+	return ret, rErr.ErrorOrNil()
 }
 
 func isLogFilteredByLevel(logFilter *model.LogFilter, entry *model.LogEntry) bool {

server/channels/app/platform/log_test.go

@@ -48,6 +48,9 @@ func TestGetMattermostLog(t *testing.T) {
 		assert.NoError(t, err)
 	})
 
+	// Set MM_LOG_PATH to allow log file reads from our temp directory
+	t.Setenv("MM_LOG_PATH", dir)
+
 	// Enable log file but point to an empty directory to get an error trying to read the file
 	th.Service.UpdateConfig(func(cfg *model.Config) {
 		*cfg.LogSettings.EnableFile = true
@@ -71,6 +74,33 @@ func TestGetMattermostLog(t *testing.T) {
 	require.NotNil(t, fileData)
 	assert.Equal(t, "mattermost.log", fileData.Filename)
 	assert.Positive(t, len(fileData.Body))
+
+	// Test path validation: FileLocation outside MM_LOG_PATH should be blocked
+	t.Run("path validation prevents reading files outside log directory", func(t *testing.T) {
+		// 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 file that would be read if validation fails
+		outsideLogLocation := config.GetLogFileLocation(outsideDir)
+		err = os.WriteFile(outsideLogLocation, []byte("secret data"), 0644)
+		require.NoError(t, err)
+
+		// Set FileLocation to the outside directory (MM_LOG_PATH is still set to 'dir')
+		th.Service.UpdateConfig(func(cfg *model.Config) {
+			*cfg.LogSettings.FileLocation = outsideDir
+		})
+
+		// Should be blocked by path validation
+		fileData, err = th.Service.GetLogFile(th.Context)
+		assert.Nil(t, fileData)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside allowed logging directory")
+	})
 }
 
 func TestGetNotificationLogFile(t *testing.T) {
@@ -91,10 +121,20 @@ func TestGetNotificationLogFile(t *testing.T) {
 	dir, err := os.MkdirTemp("", "")
 	require.NoError(t, err)
 	t.Cleanup(func() {
+		// Disable file target before cleaning up to avoid a race between
+		// removing the directory and the file getting written again.
+		th.Service.UpdateConfig(func(cfg *model.Config) {
+			*cfg.NotificationLogSettings.EnableFile = false
+		})
+		th.Service.NotificationsLogger().Flush()
+
 		err = os.RemoveAll(dir)
 		assert.NoError(t, err)
 	})
 
+	// Set MM_LOG_PATH to allow log file reads from our temp directory
+	t.Setenv("MM_LOG_PATH", dir)
+
 	// Enable notifications file but point to an empty directory to get an error trying to read the file
 	th.Service.UpdateConfig(func(cfg *model.Config) {
 		*cfg.NotificationLogSettings.EnableFile = true
@@ -118,6 +158,33 @@ func TestGetNotificationLogFile(t *testing.T) {
 	require.NotNil(t, fileData)
 	assert.Equal(t, "notifications.log", fileData.Filename)
 	assert.Positive(t, len(fileData.Body))
+
+	// Test path validation: FileLocation outside MM_LOG_PATH should be blocked
+	t.Run("path validation prevents reading files outside log directory", func(t *testing.T) {
+		// 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 file that would be read if validation fails
+		outsideLogLocation := config.GetNotificationsLogFileLocation(outsideDir)
+		err = os.WriteFile(outsideLogLocation, []byte("secret data"), 0644)
+		require.NoError(t, err)
+
+		// Set FileLocation to the outside directory (MM_LOG_PATH is still set to 'dir')
+		th.Service.UpdateConfig(func(cfg *model.Config) {
+			*cfg.NotificationLogSettings.FileLocation = outsideDir
+		})
+
+		// Should be blocked by path validation
+		fileData, err = th.Service.GetNotificationLogFile(th.Context)
+		assert.Nil(t, fileData)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "outside allowed logging directory")
+	})
 }
 
 func TestGetAdvancedLogs(t *testing.T) {
@@ -134,6 +201,9 @@ func TestGetAdvancedLogs(t *testing.T) {
 			require.NoError(t, err)
 		})
 
+		// Set MM_LOG_PATH to allow advanced logging to write to our temp directory
+		t.Setenv("MM_LOG_PATH", dir)
+
 		// Setup log files for each setting
 		optLDAP := map[string]string{
 			"filename": path.Join(dir, "ldap.log"),
@@ -243,9 +313,10 @@ func TestGetAdvancedLogs(t *testing.T) {
 		require.NotNil(t, notifFile)
 		testlib.AssertLog(t, bytes.NewBuffer(notifFile.Body), mlog.LvlInfo.Name, "Some Notification")
 	})
-	// Disable AdvancedLoggingJSON
+	// Disable AdvancedLoggingJSON for all log settings
 	th.Service.UpdateConfig(func(c *model.Config) {
 		c.LogSettings.AdvancedLoggingJSON = nil
+		c.NotificationLogSettings.AdvancedLoggingJSON = nil
 	})
 	t.Run("No logs returned when AdvancedLoggingJSON is empty", func(t *testing.T) {
 		// Confirm no logs get returned
@@ -253,4 +324,119 @@ func TestGetAdvancedLogs(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, fileDatas, 0)
 	})
+
+	t.Run("path validation prevents reading files outside log directory", func(t *testing.T) {
+		// Create a temporary directory to use as the log root
+		logDir, err := os.MkdirTemp("", "logs")
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			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 file outside the log directory that should not be accessible
+		outsideDir, err := os.MkdirTemp("", "outside")
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			err = os.RemoveAll(outsideDir)
+			require.NoError(t, err)
+		})
+
+		secretFile := path.Join(outsideDir, "secret.txt")
+		err = os.WriteFile(secretFile, []byte("secret data"), 0644)
+		require.NoError(t, err)
+
+		// Create a valid log file inside the log directory
+		validLog := path.Join(logDir, "valid.log")
+		err = os.WriteFile(validLog, []byte("valid log data"), 0644)
+		require.NoError(t, err)
+
+		// Test 1: Attempt to read file outside log directory using absolute path
+		optOutside := map[string]string{
+			"filename": secretFile,
+		}
+		dataOutside, err := json.Marshal(optOutside)
+		require.NoError(t, err)
+
+		logCfgOutside := mlog.LoggerConfiguration{
+			"malicious": mlog.TargetCfg{
+				Type:    "file",
+				Format:  "json",
+				Levels:  []mlog.Level{mlog.LvlError},
+				Options: dataOutside,
+			},
+		}
+		logCfgDataOutside, err := json.Marshal(logCfgOutside)
+		require.NoError(t, err)
+
+		th.Service.UpdateConfig(func(c *model.Config) {
+			c.LogSettings.AdvancedLoggingJSON = logCfgDataOutside
+		})
+
+		fileDatas, err := th.Service.GetAdvancedLogs(th.Context)
+		// Should return error indicating path is outside allowed directory
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "outside allowed logging directory")
+		require.Len(t, fileDatas, 0)
+
+		// Test 2: Attempt path traversal attack
+		traversalPath := path.Join(logDir, "..", "..", "etc", "passwd")
+		optTraversal := map[string]string{
+			"filename": traversalPath,
+		}
+		dataTraversal, err := json.Marshal(optTraversal)
+		require.NoError(t, err)
+
+		logCfgTraversal := mlog.LoggerConfiguration{
+			"traversal": mlog.TargetCfg{
+				Type:    "file",
+				Format:  "json",
+				Levels:  []mlog.Level{mlog.LvlError},
+				Options: dataTraversal,
+			},
+		}
+		logCfgDataTraversal, err := json.Marshal(logCfgTraversal)
+		require.NoError(t, err)
+
+		th.Service.UpdateConfig(func(c *model.Config) {
+			c.LogSettings.AdvancedLoggingJSON = logCfgDataTraversal
+		})
+
+		fileDatas, err = th.Service.GetAdvancedLogs(th.Context)
+		// Should return error for path traversal attempt
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "outside")
+		require.Len(t, fileDatas, 0)
+
+		// Test 3: Valid path within log directory should work
+		optValid := map[string]string{
+			"filename": validLog,
+		}
+		dataValid, err := json.Marshal(optValid)
+		require.NoError(t, err)
+
+		logCfgValid := mlog.LoggerConfiguration{
+			"valid": mlog.TargetCfg{
+				Type:    "file",
+				Format:  "json",
+				Levels:  []mlog.Level{mlog.LvlError},
+				Options: dataValid,
+			},
+		}
+		logCfgDataValid, err := json.Marshal(logCfgValid)
+		require.NoError(t, err)
+
+		th.Service.UpdateConfig(func(c *model.Config) {
+			c.LogSettings.AdvancedLoggingJSON = logCfgDataValid
+		})
+
+		fileDatas, err = th.Service.GetAdvancedLogs(th.Context)
+		require.NoError(t, err)
+		require.Len(t, fileDatas, 1)
+		require.Equal(t, "valid.log", fileDatas[0].Filename)
+		require.Equal(t, []byte("valid log data"), fileDatas[0].Body)
+	})
 }

server/channels/app/platform/support_packet_test.go

@@ -37,6 +37,9 @@ func TestGenerateSupportPacket(t *testing.T) {
 		assert.NoError(t, err)
 	})
 
+	// Set MM_LOG_PATH to allow log file reads from our temp directory
+	t.Setenv("MM_LOG_PATH", dir)
+
 	th.Service.UpdateConfig(func(cfg *model.Config) {
 		*cfg.LogSettings.FileLocation = dir
 		*cfg.NotificationLogSettings.FileLocation = dir

server/channels/app/support_packet_test.go

@@ -36,6 +36,9 @@ func TestGenerateSupportPacket(t *testing.T) {
 		assert.NoError(t, err)
 	})
 
+	// Set MM_LOG_PATH to allow log file reads from our temp directory
+	t.Setenv("MM_LOG_PATH", dir)
+
 	th.App.UpdateConfig(func(cfg *model.Config) {
 		*cfg.LogSettings.FileLocation = dir
 		*cfg.NotificationLogSettings.FileLocation = dir

server/channels/testlib/helper.go

@@ -35,6 +35,7 @@ type MainHelper struct {
 
 	status           int
 	testResourcePath string
+	testLogsPath     string
 	replicas         []string
 	storePool        *sqlstore.TestPool
 }
@@ -88,6 +89,15 @@ func NewMainHelperWithOptions(options *HelperOptions) *MainHelper {
 		log.Fatal(err)
 	}
 
+	// Create a logs directory and set MM_LOG_PATH for tests that validate log file paths.
+	// This is done unconditionally so tests don't need to enable full resources just for logging.
+	logsDir, err := os.MkdirTemp("", "testlogs")
+	if err != nil {
+		log.Fatal("Failed to create test logs directory: " + err.Error())
+	}
+	os.Setenv("MM_LOG_PATH", logsDir)
+	mainHelper.testLogsPath = logsDir
+
 	if options != nil {
 		mainHelper.Options = *options
 
@@ -293,6 +303,10 @@ func (h *MainHelper) Close() error {
 	if h.testResourcePath != "" {
 		os.RemoveAll(h.testResourcePath)
 	}
+	if h.testLogsPath != "" {
+		os.RemoveAll(h.testLogsPath)
+		os.Unsetenv("MM_LOG_PATH")
+	}
 
 	if h.storePool != nil {
 		h.storePool.Close()

server/channels/testlib/resources.go

@@ -142,6 +142,12 @@ func SetupTestResources() (string, error) {
 		return "", errors.Wrapf(err, "failed to create client directory %s", clientDir)
 	}
 
+	logsDir := path.Join(tempDir, "logs")
+	err = os.Mkdir(logsDir, 0700)
+	if err != nil {
+		return "", errors.Wrapf(err, "failed to create logs directory %s", logsDir)
+	}
+
 	err = setupConfig(path.Join(tempDir, "config"))
 	if err != nil {
 		return "", errors.Wrap(err, "failed to setup config")