Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning to [p]set api if <>s are included in secret #6265

Open
wants to merge 5 commits into
base: V3/develop
Choose a base branch
from

Conversation

giplgwm
Copy link
Contributor

@giplgwm giplgwm commented Nov 1, 2023

Description of the changes

Closes #6253
For every given secret, check if its wrapped in <>, if it is, add a message to an embed with a warning that that secret may not be set properly. I tested this on a local instance of the bot and it seems to be working as expected. The warning is also logged

Have the changes in this PR been tested?

Yes

@github-actions github-actions bot added the Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). label Nov 1, 2023
@Flame442 Flame442 added the hacktoberfest-accepted Used to mark a PR as valid Hacktoberfest contribution. DO NOT REMOVE UNTIL END OF NOVEMBER (sic!)! label Nov 1, 2023
await ctx.bot.set_shared_api_tokens(service, **tokens)
if embed:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be always the case with this command so this check is obsolete

You merge the card.send to send both the string and the new embed by specifying keyword arguments in that function

redbot/core/core_commands.py Show resolved Hide resolved
for dict_key, token in tokens.items():
if token.startswith("<") and token.endswith(">"):
angle_bracket_warning = _(
"Your `{dict_key}` was set including the <> symbols. If this was an accident you may experience"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should explicitly clarify that it is wrapped by <> current sentence implies that < or > is found in the token/key

for dict_key, token in tokens.items():
if token.startswith("<") and token.endswith(">"):
angle_bracket_warning = _(
"Your `{dict_key}` was set including the <> symbols. If this was an accident you may experience"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For translatable strings, I would personally suggest naming the formatting variable something contextual so that translators are better aware of the translation context for example "api_service_name"

" the < and > removed"
).format(dict_key=dict_key, service=service)
log.warning(angle_bracket_warning)
embed.add_field(name="Warning", value=angle_bracket_warning)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field name should be translated

@giplgwm
Copy link
Contributor Author

giplgwm commented Nov 16, 2023

@Drapersniper I added the changes requested in your comments as another commit on the PR, thanks

@Kreusada Kreusada self-assigned this Feb 13, 2024
@Kreusada Kreusada added the Type: Enhancement Something meant to enhance existing Red features. label Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). hacktoberfest-accepted Used to mark a PR as valid Hacktoberfest contribution. DO NOT REMOVE UNTIL END OF NOVEMBER (sic!)! Type: Enhancement Something meant to enhance existing Red features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add warning to [p]set api if <>s are included in secret
4 participants