[MM-67314] Fix system bot DM restriction bypass (#35477)

When TeamSettings.RestrictDirectMessage is set to "team", the system bot could not create DM channels with users on different teams (or no shared team). This broke SendTestMessage, CheckPostReminders, and other background jobs that use an empty session context.

The existing bypass in GetOrCreateDirectChannel only covered bots owned by the current session user or a plugin. The system bot is owned by a system admin, so it failed the ownership check and hit the common-team guard.

Changes:
- Rename IsBotOwnedByCurrentUserOrPlugin to IsBotExemptFromDMRestrictions to better reflect its purpose
- Add an explicit system bot exemption (bot.Username == BotSystemBotUsername) as the first check in the function
- Add tests covering the system bot exemption with both empty and user sessions

server/channels/app/bot.go

@@ -353,14 +353,22 @@ func (a *App) GetBots(rctx request.CTX, options *model.BotGetOptions) (model.Bot
 	return bots, nil
 }
 
-// IsBotOwnedByCurrentUserOrPlugin checks if the given user ID is a bot owned by the current session's user or by a plugin.
-func (a *App) IsBotOwnedByCurrentUserOrPlugin(rctx request.CTX, userID string) (bool, *model.AppError) {
+// IsBotExemptFromDMRestrictions checks if the given user ID is a bot that is
+// exempt from the RestrictDirectMessage=team enforcement. This includes the
+// system bot, bots owned by the current session's user, and plugin-owned bots.
+func (a *App) IsBotExemptFromDMRestrictions(rctx request.CTX, userID string) (bool, *model.AppError) {
 	bot, appErr := a.GetBot(rctx, userID, false)
 	if appErr != nil {
 		return false, appErr
 	}
 
-	if bot.OwnerId == rctx.Session().UserId {
+	// The system bot must be able to send messages to any user regardless of
+	// team membership (e.g. push notification tests, post reminders, etc.)
+	if bot.Username == model.BotSystemBotUsername {
+		return true, nil
+	}
+
+	if session := rctx.Session(); session != nil && bot.OwnerId == session.UserId {
 		return true, nil
 	}
 
@@ -371,7 +379,7 @@ func (a *App) IsBotOwnedByCurrentUserOrPlugin(rctx request.CTX, userID string) (
 
 	availablePlugins, err := pluginsEnvironment.Available()
 	if err != nil {
-		return false, model.NewAppError("IsBotOwnedByCurrentUserOrPlugin", "app.plugin.get_plugins.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+		return false, model.NewAppError("IsBotExemptFromDMRestrictions", "app.plugin.get_plugins.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
 	}
 
 	pluginIDs := make(map[string]bool, len(availablePlugins))

server/channels/app/bot_test.go

@@ -980,7 +980,7 @@ func TestGetSystemBot(t *testing.T) {
 	})
 }
 
-func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
+func TestIsBotExemptFromDMRestrictions(t *testing.T) {
 	mainHelper.Parallel(t)
 	t.Run("bot owned by current user", func(t *testing.T) {
 		th := Setup(t).InitBasic(t)
@@ -1003,7 +1003,7 @@ func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
 		require.Nil(t, err)
 
 		rctx := th.Context.WithSession(session)
-		owned, appErr := th.App.IsBotOwnedByCurrentUserOrPlugin(rctx, bot.UserId)
+		owned, appErr := th.App.IsBotExemptFromDMRestrictions(rctx, bot.UserId)
 		require.Nil(t, appErr)
 		assert.True(t, owned)
 	})
@@ -1029,7 +1029,7 @@ func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
 		require.Nil(t, err)
 
 		rctx := th.Context.WithSession(session)
-		owned, appErr := th.App.IsBotOwnedByCurrentUserOrPlugin(rctx, bot.UserId)
+		owned, appErr := th.App.IsBotExemptFromDMRestrictions(rctx, bot.UserId)
 		require.Nil(t, appErr)
 		assert.False(t, owned)
 	})
@@ -1044,7 +1044,7 @@ func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
 		require.Nil(t, err)
 
 		rctx := th.Context.WithSession(session)
-		owned, appErr := th.App.IsBotOwnedByCurrentUserOrPlugin(rctx, model.NewId())
+		owned, appErr := th.App.IsBotExemptFromDMRestrictions(rctx, model.NewId())
 		require.NotNil(t, appErr)
 		assert.False(t, owned)
 		require.Equal(t, "store.sql_bot.get.missing.app_error", appErr.Id)
@@ -1072,7 +1072,7 @@ func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
 		require.Nil(t, err)
 
 		rctx := th.Context.WithSession(session)
-		owned, appErr := th.App.IsBotOwnedByCurrentUserOrPlugin(rctx, bot.UserId)
+		owned, appErr := th.App.IsBotExemptFromDMRestrictions(rctx, bot.UserId)
 		require.Nil(t, appErr)
 		assert.False(t, owned)
 	})
@@ -1118,8 +1118,32 @@ func TestIsBotOwnedByCurrentUserOrPlugin(t *testing.T) {
 		require.Nil(t, err)
 
 		rctx := th.Context.WithSession(session)
-		owned, appErr := th.App.IsBotOwnedByCurrentUserOrPlugin(rctx, bot.UserId)
+		owned, appErr := th.App.IsBotExemptFromDMRestrictions(rctx, bot.UserId)
 		require.Nil(t, appErr)
 		assert.True(t, owned)
 	})
+
+	t.Run("system bot is always exempt regardless of session", func(t *testing.T) {
+		th := Setup(t).InitBasic(t)
+
+		systemBot, appErr := th.App.GetSystemBot(th.Context)
+		require.Nil(t, appErr)
+
+		// Exempt even with an empty context (background job with no session)
+		exempt, appErr := th.App.IsBotExemptFromDMRestrictions(th.Context, systemBot.UserId)
+		require.Nil(t, appErr)
+		assert.True(t, exempt)
+
+		// Exempt even when the session belongs to an unrelated non-admin user
+		session, err := th.App.CreateSession(th.Context, &model.Session{
+			UserId: th.BasicUser.Id,
+			Roles:  th.BasicUser.GetRawRoles(),
+		})
+		require.Nil(t, err)
+		rctx := th.Context.WithSession(session)
+
+		exempt, appErr = th.App.IsBotExemptFromDMRestrictions(rctx, systemBot.UserId)
+		require.Nil(t, appErr)
+		assert.True(t, exempt)
+	})
 }

server/channels/app/channel.go

@@ -329,21 +329,21 @@ func (a *App) GetOrCreateDirectChannel(rctx request.CTX, userID, otherUserID str
 		if err != nil {
 			return nil, err
 		}
-		var isPluginOwnedBot bool
+		var isBotExempt bool
 		for _, user := range users {
 			if user.IsBot {
-				isOwnedByCurrentUserOrPlugin, err := a.IsBotOwnedByCurrentUserOrPlugin(rctx, user.Id)
+				exempt, err := a.IsBotExemptFromDMRestrictions(rctx, user.Id)
 				if err != nil {
 					return nil, err
 				}
-				if isOwnedByCurrentUserOrPlugin {
-					isPluginOwnedBot = true
+				if exempt {
+					isBotExempt = true
 					break
 				}
 			}
 		}
-		// if one of the users is a bot, don't restrict to team members
-		if !isPluginOwnedBot {
+		// if one of the users is an exempt bot, don't restrict to team members
+		if !isBotExempt {
 			commonTeamIDs, err := a.GetCommonTeamIDsForTwoUsers(userID, otherUserID)
 			if err != nil {
 				return nil, err
@@ -3815,11 +3815,11 @@ func (a *App) getDirectOrGroupMessageMembersCommonTeams(rctx request.CTX, reques
 	userIDs := make([]string, 0, len(users))
 	for _, user := range users {
 		if user.IsBot {
-			isOwnedByCurrentUserOrPlugin, err := a.IsBotOwnedByCurrentUserOrPlugin(rctx, user.Id)
+			exempt, err := a.IsBotExemptFromDMRestrictions(rctx, user.Id)
 			if err != nil {
 				return nil, err
 			}
-			if isOwnedByCurrentUserOrPlugin {
+			if exempt {
 				continue
 			}
 		}

server/channels/app/channel_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks"
 )
 
@@ -538,7 +539,7 @@ func TestGetOrCreateDirectChannel(t *testing.T) {
 			cfg.TeamSettings.RestrictDirectMessage = &setting
 		})
 
-		// Create a session for the bot owner so IsBotOwnedByCurrentUserOrPlugin can work
+		// Create a session for the bot owner so IsBotExemptFromDMRestrictions can work
 		session, err := th.App.CreateSession(th.Context, &model.Session{
 			UserId: th.BasicUser.Id,
 			Roles:  th.BasicUser.GetRawRoles(),
@@ -557,6 +558,35 @@ func TestGetOrCreateDirectChannel(t *testing.T) {
 		require.Nil(t, appErr)
 	})
 
+	t.Run("System bot can DM any user with RestrictDirectMessage=team (MM-67314)", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			setting := model.DirectMessageTeam
+			cfg.TeamSettings.RestrictDirectMessage = &setting
+		})
+
+		systemBot, appErr := th.App.GetSystemBot(th.Context)
+		require.Nil(t, appErr)
+
+		// Simulate a background job (e.g. CheckPostReminders) that uses an empty session.
+		// The system bot is on no teams, so without the fix this would return an error.
+		emptyCtx := request.EmptyContext(th.App.Log())
+		channel, appErr := th.App.GetOrCreateDirectChannel(emptyCtx, user1.Id, systemBot.UserId)
+		require.Nil(t, appErr)
+		require.NotNil(t, channel)
+
+		// Simulate a regular user triggering SendTestMessage from a non-admin session.
+		session, err := th.App.CreateSession(th.Context, &model.Session{
+			UserId: user1.Id,
+			Roles:  user1.GetRawRoles(),
+		})
+		require.Nil(t, err)
+		rctx := th.Context.WithSession(session)
+
+		channel, appErr = th.App.GetOrCreateDirectChannel(rctx, user1.Id, systemBot.UserId)
+		require.Nil(t, appErr)
+		require.NotNil(t, channel)
+	})
+
 	t.Run("User from other team cannot create with restriction", func(t *testing.T) {
 		th.App.UpdateConfig(func(cfg *model.Config) {
 			setting := model.DirectMessageTeam