[APPROVED] TechCogsV3

Applications
#1

Discord Name: eragon5779#5779

GitHub Repository (Must be V3): https://gitlab.com/Eragon5779/TechCogsV3

Description:

A collection of useful cogs for the tech-savvy

#2

I assume the “cpucompare” cog does not have a code execution vulnerability?

#3

I don’t believe so. I haven’t tried everything possible, but I don’t pass user strings in except as searches through a dictionary, so it would be extremely difficult at best to arbitrarily run code. A friend and I have tried everything we could think of and nothing worked, if that helps.

#4

Okay, thanks for the clarification.

#5

Unless you are a co-author on this project, please refrain from commenting on another user’s application. If you suspect to have found a vulnerability or something nefarious then message QA directly here: https://cogboard.discord.red/groups/Quality-Assurance

#6
#7

Sorry for the delay eragon. I’ll be conducting the review for your application soon. You have one person in front of you at the moment, and then I’ll tackle what you got. Again, sorry for the delay.

1 Like
#8

Hello again. The following findings are from my preliminary review of the application. I’ll be doing a more in-depth final review at a later date. Please let me know if you have any questions.

cpucompare

  • Installation message needs to disclose that this cog comes bundled with some data.
  • CPUCompare class is missing a docstring.
  • cpureload command is missing a docstring.

encode

  • Class name, encode, should be capitalized
  • Main class is missing a docstring.
  • Several instances of pass_context=True should be removed. (This is default behavior now)
  • b64, b16, b32, a85, b85,for decode and encode require docstrings.

hash

  • class name, has, should be capitalized.
  • Main class is missing a docstring.
  • Several subcommands under hash require a docstring.
  • Several instances of pass_context=True should be removed. (This is default behavior now)
#9

Here are the changes I have made based on your preliminary review:

Repo

  • Added all missing docstrings
  • Capitilized all classes
  • Removed pass_context=True

cpucompare

  • Added installation message mentioning bundled data
  • Removed cpureload command (was a legacy command that is no longer needed)

If you have a better/preferred way of mentioning bundled data, please let me know!

#10

Hey Eragon, sorry for the wait. Here is my final review:

CPUCompare

  • loading the bundled data path in setup is no longer necessary and has been deprecated. You don’t need to change anything else in relation to the use of bundled data, just remove the references in your setup. (This will it no longer necessary for your setup to be async)

  • I kept getting “No processors found”, even though it did find a processor for me. I was still able to select one to show the data, but there is likely a bug when checking the length of cpuList. Here was my output:

wmkVIcU.png637Ă—531 39.4 KB

When you are creating the embed from line 94 to 114, you can remove the inline=True argument, since that is the default.

Hash

You should be reusing the session from aiohttp here. It will look something like this:

def __init__(self, *args, **kwargs): 
        self._session = aiohttp.ClientSession()

# Closes the session when the cog is unloaded.
def __unload(self):
    asyncio.get_event_loop().create_task(self._session.close())

Your get data will then make a call to self._session when you get the retrieve the url.

Encode

:+1:

Main

I’m not sure I understand the purpose of the intelreprocess.py and specdbdump.py files. If you plan to remove keys from these large json files, you should do it as an update, and not require the user to use these.

Let me know if you have any questions.

#11

Changes made:

CPUCompare

  • Removed data loading to match new methodology, setup no longer async
  • I can’t seem to replicate this issue? I tried numerous ways beyond the screenshot below to no avail:
    image
    image.png611Ă—1000 85.8 KB
  • Removed inline=True on all embeds

Hash

  • Cog now initiates session on __init__, and uses single session for all requests

Main

intelreprocess.py and specdbdump.py are simply the utilities I use when I do data updates. No manual updates beyond the normal are required on the user side. If these need to be removed, I will gladly do so, as they are just utilities.

#12

Eragon, I have to apologize. I looked back over the test I ran and realized I messed up. I accidentally used the intel command twice, and the “No processors found” was the result from the first message. (I didn’t realize you could just reply with the model). Sorry for any trouble or hair pulling this may have caused.

I appreciate all the changes you made! I don’t think those utilities are necessary for those downloading the repository, so I’ll approve the application, pending their removal.

#13

Removed the utilities!

#14
#15

Thank you for the swift response. I have approved your application. Congratulations on becoming a cog creator! Please note that it may take up to 24 hours for all of perks to be visible and for our listing to be updated. If you have any questions please feel free to contact me or another QA member in a DM on discord.

#16
#17
#18