[APPROVED] [Grommish-Cogs] Repo for VirusTotal

Applications
1

Discord Name:
gamingwithgeezers (736848614378176543)
GitHub Repository (Must be V3):

Description:
Single Cog at the moment to scan URLs/IPs sent to a given Guild against VirusTotal API (https://www.virustotal.com) for Malicious/Suspicious links. l

2

Updated code to clean up VirusTotal,
Added README.md files and some info.json files

3

Updated link checking and reporting - Now displays the engines that hit positive.
Updated README.md

image

5

Grommish-Cogs

  • Repository Review: Grommish/Grommish-Cogs
  • Commit hash at time of review: cc6ac913f5c330541652454671522a3042d0af4e

Hi Grommish, thank you for your patience whilst I reviewed your repository. If you have any queries about this review, please send me a message on Discord.

General Comments

  • When getting API keys, you should use bot.get_shared_api_tokens. Users should use [p]set api to configure their tokens for virustotal. You should update your cog’s info.json to inform users about this.
  • Your code is very neat and tidy, well commented and well structured. I like to see this because good readability means easier maintainability.
  • You have great knowledge about discord.py and Red’s API.
  • Your error handling is excellent. Most calls are handled with caution, and exceptions are logged when necessary.
  • It might be easier for future maintenance to use type-hinting when coding, so your IDE understands what types your objects are, allowing for more fluent code writing.
  • In your repo and cog’s info.json, the tags field should consist of a list of tags, which are descriptive of the cog. They help to tailor index search results to your cog. You could use words like moderation, scanning, or ban… you get the idea.

Review Comments - info.json (cog)

  • The end_user_data_statement key is missing, which is required to be fully EUD compliant. It should disclose whether, how and/or why, data is stored about users.

Review Comments - __init__.py

  • __red_end_user_data_statement__ is missing, which is required to be fully EUD compliant. It should disclose whether, how and/or why, data is stored about users.

Review Comments - virustotal.py

  • L14: You import aiohttp on line 14, but never use it. It should be used in favour of requests.
  • L16: You import requests on line 16. The requests library is blocking because it operates synchronously, meaning it halts the entire program’s execution while waiting for a response from the server, which can cause delays in an asynchronous environment like an operating Red instance. Instead, you should use aiohttp and instantiate a ClientSession which is bound to your cog’s class. It should also be gracefully closed on cog_unload. You should refactor line 368 for this change.
  • L24: The namespace for logging should be red.your_repo_name.cog_name.
  • L135-L137: A UX consideration, maybe a separate message should be sent if there are no excluded roles. I.e. “No roles have been excluded.”.
  • L200-L216, L255-L263, L336-L341, L419-L424: There is a large number of Config calls here. Generally, we should convert the scope into a dictionary if we want to access multiple values from it. This can be achieved by using:
    data = await self.config.guild(guild).all()
  • L344, L347, L358, L412-L415, L428, L544: Why not use log.debug here?
  • L368: This is where you’ll want to change to using aiohttp so you’re not blocking the bot. This won’t be difficult to implement as you have access to self inside an asynchronous method. Please see my comment about L16 for further details.
    async with self.session.get(url, headers=headers) as response:
    if response.status != 200:
        return # or handle
    json_response = await response.json()
    ...
  • L392-L399: If Quttera is being ignored entirely, you could consider popping it from the dictionary earlier. This prevents the need for additional checks further down the line.
    total_scanners.pop("Quttera", None)
  • L464-L473: You appear to be using printf-style string formatting in your exception messages, but aren’t providing any values to format the strings.
  • L484: in `{punishment_channel.name}` channel → in {punishment_channel.mention}.
  • L580: Perhaps unintentional, but this synchronous function is no longer needed.

Decision

Your cog is close to being ready for approval. The improvements suggested are mainly optimizations, performance improvements, and minor compliance issues. You’ve demonstrated a great understanding of discord.py, Red’s API, and Python coding practices. If you’d like any clarification on any points, do feel free to reach out on Discord.

6

Hello!

I have resolved the outstanding issues. I appreciate your time and patience.

7

All set, marking this repository as approved!