From 7abc9bdcf16c9d844978f7743aa86d664deeabef Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Wed, 20 Oct 2021 12:13:07 +0200 Subject: [PATCH] Pre-fetch app owners and fail early on no owners (#4926) * Pre-fetch app owners and fail early on no owners * Improve command mention in error message * Further change the order of startup actions --- redbot/__main__.py | 22 ++++++++++++-- redbot/core/bot.py | 69 ++++++++++++++++++++++++------------------- redbot/core/events.py | 18 +---------- 3 files changed, 59 insertions(+), 50 deletions(-) diff --git a/redbot/__main__.py b/redbot/__main__.py index 4b3f17cb4..e32d54bb2 100644 --- a/redbot/__main__.py +++ b/redbot/__main__.py @@ -28,7 +28,7 @@ from redbot import _early_init, __version__ _early_init() import redbot.logging -from redbot.core.bot import Red, ExitCodes +from redbot.core.bot import Red, ExitCodes, _NoOwnerSet from redbot.core.cli import interactive_config, confirm, parse_cli_flags from redbot.setup import get_data_dir, get_name, save_config from redbot.core import data_manager, drivers @@ -384,7 +384,7 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: await red.http.close() sys.exit(0) try: - await red.start(token, bot=True, cli_flags=cli_flags) + await red.start(token, bot=True) except discord.LoginFailure: log.critical("This token doesn't seem to be valid.") db_token = await red._config.token() @@ -403,6 +403,24 @@ async def run_bot(red: Red, cli_flags: Namespace) -> None: style="red", ) sys.exit(1) + except _NoOwnerSet: + print( + "Bot doesn't have any owner set!\n" + "This can happen when your bot's application is owned by team" + " as team members are NOT owners by default.\n\n" + "Remember:\n" + "ONLY the person who is hosting Red should be owner." + " This has SERIOUS security implications." + " The owner can access any data that is present on the host system.\n" + "With that out of the way, depending on who you want to be considered as owner," + " you can:\n" + "a) pass --team-members-are-owners when launching Red" + " - in this case Red will treat all members of the bot application's team as owners\n" + f"b) set owner manually with `redbot --edit {cli_flags.instance_name}`\n" + "c) pass owner ID(s) when launching Red with --owner" + " (and --co-owner if you need more than one) flag\n" + ) + sys.exit(1) return None diff --git a/redbot/core/bot.py b/redbot/core/bot.py index 89fdb8467..1075f5e6c 100644 --- a/redbot/core/bot.py +++ b/redbot/core/bot.py @@ -75,6 +75,10 @@ def _is_submodule(parent, child): return parent == child or child.startswith(parent + ".") +class _NoOwnerSet(RuntimeError): + """Raised when there is no owner set for the instance that is trying to start.""" + + # Order of inheritance here matters. # d.py autoshardedbot should be at the end # all of our mixins should happen before, @@ -217,8 +221,8 @@ class Red( self._main_dir = bot_dir self._cog_mgr = CogManager() self._use_team_features = cli_flags.use_team_features - # to prevent multiple calls to app info in `is_owner()` - self._app_owners_fetched = False + # to prevent multiple calls to app info during startup + self._app_info = None super().__init__(*args, help_command=None, **kwargs) # Do not manually use the help formatter attribute here, see `send_help_for`, # for a documented API. The internals of this object are still subject to change. @@ -1066,15 +1070,15 @@ class Red( # end Config migrations - async def pre_flight(self, cli_flags): + async def _pre_login(self) -> None: """ - This should only be run once, prior to connecting to discord. + This should only be run once, prior to logging in to Discord REST API. """ await self._maybe_update_config() self.description = await self._config.description() init_global_checks(self) - init_events(self, cli_flags) + init_events(self, self._cli_flags) if self._owner_id_overwrite is None: self._owner_id_overwrite = await self._config.owner() @@ -1086,9 +1090,13 @@ class Red( i18n_regional_format = await self._config.regional_format() i18n.set_regional_format(i18n_regional_format) + async def _pre_connect(self) -> None: + """ + This should only be run once, prior to connecting to Discord gateway. + """ self.add_cog(Core(self)) self.add_cog(CogManagerUI()) - if cli_flags.dev: + if self._cli_flags.dev: self.add_cog(Dev()) await modlog._init(self) @@ -1119,11 +1127,11 @@ class Red( ) python_version_changed = True else: - if cli_flags.no_cogs is False: + if self._cli_flags.no_cogs is False: packages.extend(await self._config.packages()) - if cli_flags.load_cogs: - packages.extend(cli_flags.load_cogs) + if self._cli_flags.load_cogs: + packages.extend(self._cli_flags.load_cogs) system_changed = False machine = platform.machine() @@ -1189,13 +1197,29 @@ class Red( if self.rpc_enabled: await self.rpc.initialize(self.rpc_port) + async def _pre_fetch_owners(self) -> None: + app_info = await self.application_info() + + if app_info.team: + if self._use_team_features: + self.owner_ids.update(m.id for m in app_info.team.members) + elif self._owner_id_overwrite is None: + self.owner_ids.add(app_info.owner.id) + + self._app_info = app_info + + if not self.owner_ids: + raise _NoOwnerSet("Bot doesn't have any owner set!") + async def start(self, *args, **kwargs): """ - Overridden start which ensures cog load and other pre-connection tasks are handled + Overridden start which ensures that cog load and other pre-connection tasks are handled. """ - cli_flags = kwargs.pop("cli_flags") - await self.pre_flight(cli_flags=cli_flags) - return await super().start(*args, **kwargs) + await self._pre_login() + await self.login(*args) + await self._pre_fetch_owners() + await self._pre_connect() + await self.connect() async def send_help_for( self, @@ -1279,24 +1303,7 @@ class Red( ------- bool """ - if user.id in self.owner_ids: - return True - - ret = False - if not self._app_owners_fetched: - app = await self.application_info() - if app.team: - if self._use_team_features: - ids = {m.id for m in app.team.members} - self.owner_ids.update(ids) - ret = user.id in ids - elif self._owner_id_overwrite is None: - owner_id = app.owner.id - self.owner_ids.add(owner_id) - ret = user.id == owner_id - self._app_owners_fetched = True - - return ret + return user.id in self.owner_ids async def is_admin(self, member: discord.Member) -> bool: """Checks if a member is an admin of their guild.""" diff --git a/redbot/core/events.py b/redbot/core/events.py index db7f3606f..d5fcafa2a 100644 --- a/redbot/core/events.py +++ b/redbot/core/events.py @@ -70,19 +70,7 @@ def init_events(bot, cli_flags): guilds = len(bot.guilds) users = len(set([m for m in bot.get_all_members()])) - app_info = await bot.application_info() - - if app_info.team: - if bot._use_team_features: - bot.owner_ids.update(m.id for m in app_info.team.members) - elif bot._owner_id_overwrite is None: - bot.owner_ids.add(app_info.owner.id) - bot._app_owners_fetched = True - - try: - invite_url = discord.utils.oauth_url(app_info.id) - except: - invite_url = "Could not fetch invite url" + invite_url = discord.utils.oauth_url(bot._app_info.id) prefixes = cli_flags.prefix or (await bot._config.prefix()) lang = await bot._config.locale() @@ -200,10 +188,6 @@ def init_events(bot, cli_flags): if rich_outdated_message: rich_console.print(rich_outdated_message) - if not bot.owner_ids: - # we could possibly exit here in future - log.warning("Bot doesn't have any owner set!") - bot._color = discord.Colour(await bot._config.color()) bot._red_ready.set() if outdated_red_message: