feat: Add PSUControl eWeLink plugin

[chrismin13]

• You have read the "Registering a new Plugin" guide.
• You want to and are able to maintain the plugin you are registering, long-term.
• You understand why the plugin you are registering works.
• You have read and acknowledge the Code of Conduct.

What is the name of your plugin?

PSUControl eWeLink

What does your plugin do?

It adds an interface to connect IoT devices that use the eWeLink app (such as Sonoff Smart Plugs) using the existing PSU Control plugin.

Where can we find the source code of your plugin?

https://github.com/chrismin13/OctoPrint-PSUControl-eWeLink

Was any kind of genAI (ChatGPT, Copilot etc) involved in creating this plugin?

Yes, a lot. I have read the guide, and I understand that "vibe coded" plugins are not acceptable. So, I understand if this will not be accepted under that premise, but I wanted to make an attempt anyway, and hope you don't mind me doing so.

However, I would like to mention that I do feel confident in my ability to maintain the plugin in case it breaks or if I need to add a small new feature. I have manually gone through everything, made sure it is safe, fully understand how the code works, and I'm confident in what I am submitting.

If you still think this does not fit your submission criteria due to the use of AI, feel free to reject this pull request.

Is your plugin commercial in nature?

No

Does your plugin rely on some cloud services?

Yes, the eWeLink cloud, which is used for the smart switches. The Privacy Policy on the GitHub repo links to their Privacy Policy for more info.

Further notes

The plugin has been tested with a Sonoff Basic R2 and on an up-to-date OctoPi installation. It would be nice to get some more testers if anyone else has any eWeLink devices hooked up to their printers :D

Thank you for your consideration! Let me know if there's anything you would like to see improved.

[jacopotediosi]

Hello @chrismin13,

I briefly reviewed your repository, these are my suggestions from a security standpoint:

• I recommend implementing is_api_protected set to True to restrict your plugin's API usage to authenticated users only.

• I noticed that in your JavaScript code you use the PNotify function several times. OctoPrint currently ships with PNotify version 3, which by default does not perform HTML encoding of notification titles and text. Since you sometimes insert untrusted data into notifications (such as device names obtained from the cloud), to prevent potential XSS vulnerabilities I recommend setting the text_escape and title_escape attributes to true in all PNotify calls, unless you explicitly need to insert HTML code. Alternatively, you can use the ._escape() function from the Lodash library (already imported by OctoPrint) when concatenating data.

I also have some doubts about encrypting and masking passwords in OctoPrint plugins: it seems like security theater to me.
I see no added security value in encrypting the password at rest, nor in masking it in the frontend, given OctoPrint's current threat model (explained below).
On the backend, the security of passwords stored by plugins is delegated to OctoPrint, which stores all configurations in its config.yaml file. If that file were exfiltrated by an attacker, it would inevitably lead to the attacker becoming an OctoPrint admin. At that point, whatever encryption you implement, nothing could prevent the attacker from obtaining the key you used to encrypt your plugin passwords.
On the frontend, password masking is already handled by the browser, which processes the HTML type="password" attribute.
In this scenario, I see password manipulation as adding complexity to the code and consequently increasing the risk of implementation errors. Other major OctoPrint plugins currently store credentials in plaintext. However, if you still want to keep your current implementation, consider that XOR is an insecure encryption mechanism; you should use at least AES or Fernet.

Thank you again for your contribution 😄

[chrismin13]

Hello @jacopotediosi, thank you for taking the time to go through my PR, I really appreciate the feedback!

I've implemented both is_api_protected and the fixes for the potential XSS vulnerabilities you mentioned.

As for the password masking, I agree about your point in regards to it being security theater. Perhaps my original way of phrasing it and explaining the implementation was not great. I understand that, at the end of the day, any data stored on the server will be accessible by an attacker, and there's nothing that can be done for that.

I only wanted to make it so that my password was not visible in plain text when going through the config, as it would otherwise be a normal username and password getting stored, and not some kind of API token or other UUID. So, yes, this is not more secure, but if some config file ever ends up in some kind of log, or if a user copy pastes a config somewhere else, it won't be an instant security issue.

EDIT: So, I forgot to clarify, I've updated all of the documentation to make it clear that this is only obfuscation, not encryption, and I've removed any mention of this from the plugin page, as I don't think it should be advertised as a feature.

If you disagree with this, I would be happy to either remove this entirely, and have the password displayed in plaintext in the OctoPrint config file, or implement AES or Fernet as you suggested.

Thanks again for your time! Let me know if you have any other questions or concerns.

[jacopotediosi]

@chrismin13 thanks for your patience.

I've reviewed the plugin code in your repository and the files in this PR again and, apart from my two comments above, now they look good to me.

About the obfuscation thing, now that the documentation is clear that it only serves to prevent casual exposure of credentials, it's ok for me.

Please note that I'm just a user here, I won't be the one merging your PR, so you'll still need to wait for a review from someone with the privileges to do so.

[chrismin13]

@jacopotediosi ah ok, thanks for clarifying! Still, your comments have been very helpful, so thanks for taking the time to look at the PR. I've addressed your latest comments as well. I'll wait for review from someone else from the team.

[jneilliii]

However, I would like to mention that I do feel confident in my ability to maintain the plugin in case it breaks or if I need to add a small new feature. I have manually gone through everything, made sure it is safe, fully understand how the code works, and I'm confident in what I am submitting.

Based on this statement, I believe we can allow the plugin to be registered. I will review the code and if anything feels off or contrary to this statement I'll let you know. From what I've seen historically from vibe coded plugins is implementations that don't quite take into account the OctoPrint plugin ecosystem well, typically related to knockout binding type issues.

[jneilliii]

one thing right out the gate I see is the old method of using setup.py instead of pyproject.toml approach which is incorporated into the latest plugin template cookiecutter.

[jneilliii]

based on your dependency of ewelink, this should be minimum python version 3.9.

[jneilliii]

same issue in your plugin...so as you can see what I meant, vibe coding is not great.

https://github.com/chrismin13/OctoPrint-PSUControl-eWeLink/blob/71e74911ecde8b8647d38c0c6fb4f102610fce5d/octoprint_psucontrol_ewelink/__init__.py#L14

[jneilliii]

Outside of the above python version issue, I don't see anything that stands out, but will typically ask @kantlivelong to review since this is a PSU Control sub-plugin.

[chrismin13]

Hello @jneilliii, thanks for taking a look at the plugin! I've corrected the Python version that you mentioned, and I've also bumped the OctoPrint version to something newer, let me know if you disagree with that. I can't personally test it on all OctoPrint versions, but I see that there were some changes related to pyproject.toml on that release, and also that 1.8.0 was the first one requiring Python 3+, so I'm thinking that it's probably a safe bet. Plus, most users shouldn't be running an older version nowadays anyway.

As for the cookie cutter template - indeed, I never started from the cookie cutter template, as originally I thought I would just quickly make the plugin from an empty folder for personal use, but eventually I saw that it was actually becoming a bigger project, so that's why I ended up publishing it.

Since you mentioned the template, I took a look at it, and tried to implement anything useful I could find from it. So, I added the GitHub issue templates, moved from setup.py to pyproject.toml, and added multi-language support with babel. I've got support for Greek and French for now, as those are the languages I can vouch for, but I'm happy to add more if anyone is interested in contributing, and I added docs with instructions on how to implement them.

Happy to adjust things as you see fit, and of course to implement any feedback that you or @kantlivelong might have.

[jacopotediosi]

I checked the recent commits in the plugin repository again. The plugin still works, but, sigh, I noticed something - and that's exactly why the policy would reject vibe-coded plugins.

1. Here the AI hallucinated, trying to implement a TranslationPlugin mixin that doesn't exist, thus breaking plugin loading.
2. In the following commit, you likely asked the AI why the plugin wasn't working anymore, and it changed the __init__ method to initialize. The initialize method exists, but it's not exactly the same as __init__. For initializing plugin variables, using initialize is weird - variables are usually initialized at class creation in __init__.
3. In this other commit the AI removed the import of the non-existent mixin, finally fixing plugin loading.

The __init__ to initialize change remained, and you likely didn't notice. It currently has no side effects, but who know what will happen in the future.

This is exactly what's scary about genAI: for a moment you lose control, you don't really know what's happening, and you end up relying solely on the AI.

The good thing is that this plugin is useful and probably wouldn't have been created without AI, so it's still a win for me.

[chrismin13]

I'm glad to see you're going through the commits. Since you're looking at the commit history, it's important to notice the timeline. I'm not committing any changes without manually reviewing the code first and not testing it as well. I am aware of any changes being made, and I try to understand the changes that happen.

The TranslationPlugin stuff was a weirdly consistent hallucination, persisting multiple sessions. It happened while I was trying to debug some other issues. Since I couldn't find any other documentation online about translating OctoPrint plugins apart from what was in the Cookie Cutter template, I decided to give it a shot. I did end up removing it when I had issues loading my plugin, and double checked which classes are available for future reference.

As for the __init__ to initialize change, I decided to keep that in, as it seems like that is a good change in my eyes. It runs after everything else has already been initialized, and does not override the __init__ function, so it's less likely that I'm going to run into issues with other things not having loaded. From what I understand, and from the research that I did on this, if I were to stick to using __init__, it would be best to start using super.__init__() at the end of my original function, to keep the rest of the Mixins that I'm using functioning correctly. Whereas now, with initialize, I can use that and not have to worry about having to pass properties around.

Let me know if my understanding of this is wrong.

[foosel]

When it comes to available plugin mixins and hooks and such, I would strongly advise to refer to the available documentation rather than blindly trusting a stochastic parrot.

As for the __init__ to initialize change, I decided to keep that in, as it seems like that is a good change in my eyes. It runs after everything else has already been initialized, and does not override the __init__ function, so it's less likely that I'm going to run into issues with other things not having loaded. From what I understand, and from the research that I did on this, if I were to stick to using __init__, it would be best to start using super.__init__() at the end of my original function, to keep the rest of the Mixins that I'm using functioning correctly. Whereas now, with initialize, I can use that and not have to worry about having to pass properties around.

Valid points, you are however going against best practices of Python development that way.

__init__ is the class's constructor. It should declare all variables that are available on the class. By not defining it and only declaring the variables in your OctoPrint-specific initialize, you risk bugs arising from calling your class during loading and initialization, break code analysis tooling and typing support and in general make your code less readable.

super().__init__ is there for precisely the reason you pointed out, to call the parent class constructor(s). Contrary to other languages it isn't automatically called for you if absent from your constructor, but that also gives you way more control over initialization order and functionality overrides. A good IDE will actually just prefill super().__init__ for you upon detecting that you are writing a constructor. Tbh, I'm kinda surprised that the LLM you are utilizing here apparently doesn't do that.

tl;dr: What you do there with initialize works, but it goes against best practices and should be avoided in code that you plan on keeping on using and maintaining.

[chrismin13]

Hi @foosel, thanks for the info! I've updated the code accordingly. I'll refer to the documentation going forwards.