RFC Discussion: rfc-05: Disallow downloading and executing code

[gremlation]

This is the discussion thread for RFC PR #12.

Please provide feedback and discuss the RFC here rather than in the PR comments.

PR Link: #12

[gremlation]

Existing precedent in Apple's App Store guidelines:

2.5.2 Apps should be self-contained in their bundles, and may not read or write data outside the designated container area, nor may they download, install, or execute code which introduces or changes features or functionality of the app, including other apps.

[teward]

Note also we already have a precedent on this in the current policy - we disallow the independent execution of pip install which can install separate repos and URLs and libraries that are outside auditable scope or the control of ComfyUI-Manager. This is essentially an extension of that policy.

[geroldmeisinger]

If you only disallow certain packages, like requests, aiohttp, axios, what prevents a malicious actor to include another package which provides the same features of "downloading"? And just upload it on pypl?
What prevents a malicious actor to call into javascript context, make a XHttpRequest there, and send it back to python context? (xss and cors probably)
What prevents a malicious actor to call pathlib.Path.unlink(), shutil.rmtree(), os.remove(), os.unlink() etc.?

I think ComfyUI should provide an official containerization instead! There a many examples with Docker floating around, but an official one would find wider adoption.

There are multiple custom nodes which provide downloading via HTTP requests as a node and there is utility here, e.g. https://github.com/wawahuy/ComfyUI-HTTP. But we could provide a setting to enable HTTP requests, make it disabled by default, and optionally allow it on a per user or per workflow basis.

Arbitrary code execution is also an issue that all the "python nodes" have to deal with:

• python-interpreter-node
• ASTERR "ASTERR runs Python Code from a Web Interface! It is highly recommended to run this in a closed-off environment, as it could have potential security risks."
• anyPython "Some modules (like os, sys, subprocess) are flagged as risky and require risk confirmation."
• AnyNode "The way that AnyNode works, is that it executes code which happens externally from python that is coming back from the server on a ChatCompletions endpoint. To put that into perspective, wherever you point it, you are giving some sort of control in python to that place. BE CAREFUL that if you are not pointing it to localhost that you absolutely trust the address that you put into server."

Existing precedent in Apple's App Store guidelines

I believe Apple's motivation behind this guideline was to have control over the apps feature for legal reasons (age rating, NSFW, circumvention of payment services), not for security reasons. Security is provided by sandboxing.

[geroldmeisinger]

Another incident reported on reddit: https://www.reddit.com/r/comfyui/comments/1qn4w1j/i_think_my_comfyui_has_been_compromised_check_in/