From a58af531a48e473a39f17105615439a2f9dd94f0 Mon Sep 17 00:00:00 2001 From: Tejas <98106526+ToxicBiohazard@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:31:11 +0530 Subject: [PATCH 1/3] Enhance ban and kick commands to require evidence. Updated methods to validate evidence input and handle missing evidence cases. Adjusted related tests to ensure proper functionality and error handling. --- src/cmds/core/ban.py | 4 +-- src/cmds/core/user.py | 7 +++-- src/helpers/ban.py | 14 +++++++-- src/webhooks/handlers/account.py | 4 +-- src/webhooks/handlers/base.py | 21 +++++++++++++ tests/src/cmds/core/test_user.py | 34 +++++++++++++++------ tests/src/helpers/test_ban.py | 13 +++++++- tests/src/webhooks/handlers/test_account.py | 29 ++++++++++++++++++ tests/src/webhooks/handlers/test_base.py | 17 +++++++++++ 9 files changed, 125 insertions(+), 18 deletions(-) diff --git a/src/cmds/core/ban.py b/src/cmds/core/ban.py index 67699ef..055eba1 100644 --- a/src/cmds/core/ban.py +++ b/src/cmds/core/ban.py @@ -28,7 +28,7 @@ def __init__(self, bot: Bot): @slash_command(guild_ids=settings.guild_ids, description="Ban a user from the server permanently.") @has_any_role(*settings.role_groups.get("ALL_ADMINS"), *settings.role_groups.get("ALL_SR_MODS")) async def ban( - self, ctx: ApplicationContext, user: discord.Member, reason: str, evidence: str = None + self, ctx: ApplicationContext, user: discord.Member, reason: str, evidence: str ) -> Interaction | WebhookMessage: """Ban a user from the server permanently.""" await ctx.defer(ephemeral=False) @@ -48,7 +48,7 @@ async def ban( *settings.role_groups.get("ALL_HTB_STAFF") ) async def tempban( - self, ctx: ApplicationContext, user: discord.Member, duration: str, reason: str, evidence: str = None + self, ctx: ApplicationContext, user: discord.Member, duration: str, reason: str, evidence: str ) -> Interaction | WebhookMessage: """Ban a user from the server temporarily.""" await ctx.defer(ephemeral=False) diff --git a/src/cmds/core/user.py b/src/cmds/core/user.py index 1e2cc10..12cc780 100644 --- a/src/cmds/core/user.py +++ b/src/cmds/core/user.py @@ -15,7 +15,7 @@ from src.core import settings from src.database.models import HtbDiscordLink from src.database.session import AsyncSessionLocal -from src.helpers.ban import add_infraction +from src.helpers.ban import add_infraction, validate_evidence from src.helpers.checks import member_is_staff logger = logging.getLogger(__name__) @@ -72,7 +72,7 @@ async def bad_name(self, ctx: ApplicationContext, user: Member) -> Interaction | @slash_command(guild_ids=settings.guild_ids, description="Kick a user from the server.") @has_any_role(*settings.role_groups.get("ALL_ADMINS"), *settings.role_groups.get("ALL_MODS")) - async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidence: str = None) \ + async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidence: str) \ -> Interaction | WebhookMessage: """Kick a user from the server.""" await ctx.defer(ephemeral=False) @@ -88,6 +88,9 @@ async def kick(self, ctx: ApplicationContext, user: Member, reason: str, evidenc if ctx.user.id == member.id: return await ctx.followup.send("You cannot kick yourself.") + if evidence_error := validate_evidence(evidence): + return await ctx.followup.send(evidence_error.message, delete_after=evidence_error.delete_after) + if len(reason) == 0: reason = "No reason given..." diff --git a/src/helpers/ban.py b/src/helpers/ban.py index 763a0dc..6c8f6db 100644 --- a/src/helpers/ban.py +++ b/src/helpers/ban.py @@ -32,6 +32,16 @@ logger = logging.getLogger(__name__) +EVIDENCE_REQUIRED_MESSAGE = "Evidence is required." + + +def validate_evidence(evidence: str) -> SimpleResponse | None: + """Return an error response when evidence is missing or blank.""" + if not evidence or not evidence.strip(): + return SimpleResponse(message=EVIDENCE_REQUIRED_MESSAGE, delete_after=15) + return None + + class BanCodes(Enum): SUCCESS = "SUCCESS" ALREADY_EXISTS = "ALREADY_EXISTS" @@ -291,8 +301,8 @@ async def ban_member_with_epoch( if len(reason) == 0: reason = "No reason given ..." - if not evidence: - evidence = "none provided" + if evidence_error := validate_evidence(evidence): + return evidence_error # Validate epoch time is in the future current_time = datetime.now(tz=timezone.utc).timestamp() diff --git a/src/webhooks/handlers/account.py b/src/webhooks/handlers/account.py index 517f4a3..8b43748 100644 --- a/src/webhooks/handlers/account.py +++ b/src/webhooks/handlers/account.py @@ -96,7 +96,7 @@ async def _handle_account_banned(self, body: WebhookBody, bot: Bot) -> dict: discord_id, account_id = self.validate_common_properties(body) expires_at = self.validate_property(self.get_property_or_trait(body, "expires_at"), "expires_at") reason = body.properties.get("reason") - notes = body.properties.get("notes") + notes = self.validate_non_empty_string(body.properties.get("notes"), "notes") created_by = body.properties.get("created_by") expires_ts = int(datetime.fromisoformat(expires_at).timestamp()) # type: ignore @@ -114,7 +114,7 @@ async def _handle_account_banned(self, body: WebhookBody, bot: Bot) -> dict: member=member, expires_timestamp=expires_ts, reason=f"Platform Ban - {reason}", - evidence=notes or "N/A", + evidence=notes, author_name=created_by or "System", expires_at_str=expires_at, # type: ignore log_channel_id=settings.channels.BOT_LOGS, diff --git a/src/webhooks/handlers/base.py b/src/webhooks/handlers/base.py index aebcacb..712b708 100644 --- a/src/webhooks/handlers/base.py +++ b/src/webhooks/handlers/base.py @@ -74,6 +74,27 @@ def validate_property(self, property: T | None, name: str) -> T: return property + def validate_non_empty_string(self, value: str | None, name: str) -> str: + """ + Validates a string property is present and not blank. + + Args: + value (str | None): The string to validate. + name (str): The name of the property. + + Returns: + str: The validated string. + + Raises: + HTTPException: If the value is None or blank (400) + """ + if value is None or not str(value).strip(): + msg = f"Invalid {name}" + self.logger.debug(msg) + raise HTTPException(status_code=400, detail=msg) + + return str(value) + def validate_discord_id(self, discord_id: str | int | None) -> int | str: """ Validates the Discord ID. See validate_property function. diff --git a/tests/src/cmds/core/test_user.py b/tests/src/cmds/core/test_user.py index 92eab40..243756b 100644 --- a/tests/src/cmds/core/test_user.py +++ b/tests/src/cmds/core/test_user.py @@ -35,11 +35,11 @@ async def test_kick_success(self, ctx, guild, bot, session): patch('src.cmds.core.user.member_is_staff', return_value=False) ): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules") + await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules", "Some evidence") reason = "Violation of rules" add_infraction_mock.assert_called_once_with( - ctx.guild, user_to_kick, 0, f"Previously kicked for: {reason} - Evidence: None", ctx.user + ctx.guild, user_to_kick, 0, f"Previously kicked for: {reason} - Evidence: Some evidence", ctx.user ) # Assertions @@ -59,7 +59,7 @@ async def test_kick_fail_user_left(self, ctx, guild, bot, session): with patch('src.cmds.core.user.member_is_staff', return_value=False): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules") + await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules", "Some evidence") bot.get_member_or_user.assert_called_once_with(ctx.guild, user_to_kick.id) ctx.guild.kick.assert_not_called() @@ -76,7 +76,7 @@ async def test_kick_fail_user_not_found(self, ctx, guild, bot, session): with patch('src.cmds.core.user.member_is_staff', return_value=False): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules") + await cog.kick.callback(cog, ctx, user_to_kick, "Violation of rules", "Some evidence") bot.get_member_or_user.assert_called_once_with(ctx.guild, user_to_kick.id) ctx.guild.kick.assert_not_called() @@ -92,7 +92,7 @@ async def test_kick_fail_staff_member(self, ctx, guild, bot): with patch('src.cmds.core.user.member_is_staff', return_value=True): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, member, "Violation of rules") + await cog.kick.callback(cog, ctx, member, "Violation of rules", "Some evidence") ctx.defer.assert_awaited_once_with(ephemeral=False) ctx.followup.send.assert_called_once_with("You cannot kick another staff member.") @@ -106,7 +106,7 @@ async def test_kick_fail_bot_member(self, ctx, guild, bot): with patch('src.cmds.core.user.member_is_staff', return_value=False): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, member, "Violation of rules") + await cog.kick.callback(cog, ctx, member, "Violation of rules", "Some evidence") ctx.defer.assert_awaited_once_with(ephemeral=False) ctx.followup.send.assert_called_once_with("You cannot kick a bot.") @@ -120,11 +120,27 @@ async def test_kick_fail_self_kick(self, ctx, guild, bot): with patch('src.cmds.core.user.member_is_staff', return_value=False): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, member, "Violation of rules") + await cog.kick.callback(cog, ctx, member, "Violation of rules", "Some evidence") ctx.defer.assert_awaited_once_with(ephemeral=False) ctx.followup.send.assert_called_once_with("You cannot kick yourself.") + @pytest.mark.asyncio + async def test_kick_fail_missing_evidence(self, ctx, guild, bot): + ctx.user = helpers.MockMember(id=1, name="Test Moderator") + member = helpers.MockMember(id=2, name="User to Kick", bot=False) + ctx.guild = guild + ctx.guild.kick = AsyncMock() + bot.get_member_or_user = AsyncMock(return_value=member) + + with patch('src.cmds.core.user.member_is_staff', return_value=False): + cog = user.UserCog(bot) + await cog.kick.callback(cog, ctx, member, "Violation of rules", " ") + + ctx.defer.assert_awaited_once_with(ephemeral=False) + ctx.guild.kick.assert_not_called() + ctx.followup.send.assert_called_once_with("Evidence is required.", delete_after=15) + @pytest.mark.asyncio async def test_kick_http_exception_returns_error(self, ctx, guild, bot): ctx.user = helpers.MockMember(id=1, name="Test Moderator") @@ -139,7 +155,7 @@ async def test_kick_http_exception_returns_error(self, ctx, guild, bot): patch('src.cmds.core.user.member_is_staff', return_value=False) ): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, member, "Violation of rules") + await cog.kick.callback(cog, ctx, member, "Violation of rules", "Some evidence") ctx.defer.assert_awaited_once_with(ephemeral=False) ctx.guild.kick.assert_not_called() @@ -161,7 +177,7 @@ async def test_kick_forbidden_dm_sends_notice_and_continues(self, ctx, guild, bo patch('src.cmds.core.user.member_is_staff', return_value=False) ): cog = user.UserCog(bot) - await cog.kick.callback(cog, ctx, member, "Violation of rules") + await cog.kick.callback(cog, ctx, member, "Violation of rules", "Some evidence") ctx.defer.assert_awaited_once_with(ephemeral=False) assert ctx.followup.send.await_count == 2 diff --git a/tests/src/helpers/test_ban.py b/tests/src/helpers/test_ban.py index e39fe26..c8eae47 100644 --- a/tests/src/helpers/test_ban.py +++ b/tests/src/helpers/test_ban.py @@ -5,7 +5,7 @@ import pytest from discord import Forbidden, HTTPException -from src.helpers.ban import _check_member, _dm_banned_member, ban_member +from src.helpers.ban import EVIDENCE_REQUIRED_MESSAGE, _check_member, _dm_banned_member, ban_member from src.helpers.responses import SimpleResponse from tests import helpers @@ -150,6 +150,17 @@ async def test_ban_member_invalid_duration(self, bot, guild, member, author): assert isinstance(result, SimpleResponse) assert result.message == "Invalid duration: could not parse." + @pytest.mark.asyncio + async def test_ban_member_missing_evidence(self, bot, guild, member, author): + duration = "1d" + reason = "xf reason" + member.display_name = "Banned Member" + + with mock.patch("src.helpers.ban._check_member", return_value=None): + result = await ban_member(bot, guild, member, duration, reason, " ") + assert isinstance(result, SimpleResponse) + assert result.message == EVIDENCE_REQUIRED_MESSAGE + @pytest.mark.asyncio async def test_ban_member_permanently_success(self, bot, guild, member, author): duration = "500w" diff --git a/tests/src/webhooks/handlers/test_account.py b/tests/src/webhooks/handlers/test_account.py index e18cf7e..4a24d84 100644 --- a/tests/src/webhooks/handlers/test_account.py +++ b/tests/src/webhooks/handlers/test_account.py @@ -495,6 +495,7 @@ async def test_handle_account_banned_member_not_found(self, bot): "discord_id": discord_id, "account_id": account_id, "expires_at": expires_at, + "notes": "Repeated violations", }, traits={}, ) @@ -508,3 +509,31 @@ async def test_handle_account_banned_member_not_found(self, bot): result = await handler._handle_account_banned(body, bot) mock_log.assert_called() assert result == handler.fail() + + @pytest.mark.asyncio + async def test_handle_account_banned_missing_notes(self, bot): + """Test account banned event rejects missing notes.""" + handler = AccountHandler() + discord_id = 123456789 + account_id = 987654321 + expires_at = "2024-12-31T23:59:59" + body = WebhookBody( + platform=Platform.ACCOUNT, + event=WebhookEvent.ACCOUNT_BANNED, + properties={ + "discord_id": discord_id, + "account_id": account_id, + "expires_at": expires_at, + }, + traits={}, + ) + with ( + patch.object(handler, "validate_discord_id", return_value=discord_id), + patch.object(handler, "validate_account_id", return_value=account_id), + patch.object(handler, "validate_property", return_value=expires_at), + ): + with pytest.raises(HTTPException) as exc_info: + await handler._handle_account_banned(body, bot) + + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "Invalid notes" diff --git a/tests/src/webhooks/handlers/test_base.py b/tests/src/webhooks/handlers/test_base.py index ba774cf..0282e5e 100644 --- a/tests/src/webhooks/handlers/test_base.py +++ b/tests/src/webhooks/handlers/test_base.py @@ -100,6 +100,23 @@ def test_validate_property_none(self): assert exc_info.value.status_code == 400 assert exc_info.value.detail == "Invalid test_property" + def test_validate_non_empty_string_success(self): + """Test successful non-empty string validation.""" + handler = ConcreteHandler() + + assert handler.validate_non_empty_string("valid value", "notes") == "valid value" + + def test_validate_non_empty_string_invalid(self): + """Test non-empty string validation rejects blank values.""" + handler = ConcreteHandler() + + for invalid_value in (None, "", " "): + with pytest.raises(HTTPException) as exc_info: + handler.validate_non_empty_string(invalid_value, "notes") + + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "Invalid notes" + def test_validate_discord_id_success(self): """Test successful Discord ID validation.""" handler = ConcreteHandler() From bf8420112663ada9e0d1789f47a116a6e38b38f1 Mon Sep 17 00:00:00 2001 From: Ryan Gordon <4227041+0xRy4n@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:07:50 +0100 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/helpers/ban.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helpers/ban.py b/src/helpers/ban.py index 6c8f6db..77954c7 100644 --- a/src/helpers/ban.py +++ b/src/helpers/ban.py @@ -35,9 +35,9 @@ EVIDENCE_REQUIRED_MESSAGE = "Evidence is required." -def validate_evidence(evidence: str) -> SimpleResponse | None: +def validate_evidence(evidence: str | None) -> SimpleResponse | None: """Return an error response when evidence is missing or blank.""" - if not evidence or not evidence.strip(): + if not isinstance(evidence, str) or not evidence.strip(): return SimpleResponse(message=EVIDENCE_REQUIRED_MESSAGE, delete_after=15) return None From 669f116743f6a9ffff8d84389d62d559b3ce1ed8 Mon Sep 17 00:00:00 2001 From: Ryan Gordon <4227041+0xRy4n@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:08:05 +0100 Subject: [PATCH 3/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/webhooks/handlers/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webhooks/handlers/base.py b/src/webhooks/handlers/base.py index 712b708..d3dd3f1 100644 --- a/src/webhooks/handlers/base.py +++ b/src/webhooks/handlers/base.py @@ -88,12 +88,12 @@ def validate_non_empty_string(self, value: str | None, name: str) -> str: Raises: HTTPException: If the value is None or blank (400) """ - if value is None or not str(value).strip(): + if not isinstance(value, str) or not value.strip(): msg = f"Invalid {name}" self.logger.debug(msg) raise HTTPException(status_code=400, detail=msg) - return str(value) + return value.strip() def validate_discord_id(self, discord_id: str | int | None) -> int | str: """