Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability for extensions to provide custom extra_data to prompt #3479

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IARI
Copy link

@IARI IARI commented May 14, 2024

Use case:

An extension allows the user to store some session-specific settings in the web-ui.
A custom node needs access to that data.

See the following discussion #3476

Description of the changes

This PR adds

  • js extension method provideExtraData which allows an extension developer to provide additional data which is sent with ui-requests to the /prompt endpoint.
  • custom nodes may specify hidden inputs with type "EXTRA_DATA". the parameter name is looked up in the extra_data object from the prompt, and if there is any, that data is passed to the execution function of the custom node.

Example Code

js

app.registerExtension({
    name: "my-example-extension",
    async provideExtraData() {
        return {my_custom_data: { hello: "world"}}
    }
})

python

class MyExampleNode:
    @classmethod
    def INPUT_TYPES(s):
        return {
            "hidden": {
                "my_custom_data": "EXTRA_DATA"
            }
        }

    RETURN_TYPES = ("FLOAT",)
    CATEGORY = "example"
    FUNCTION = "extra_data_example"
    OUTPUT_NODE = False

    def extra_data_example(self, my_custom_data=None):
        if my_custom_data is None:
            print("extra_data my_custom_data is None")
        else:
            print("my_custom_data " + json.dumps(my_custom_data, indent=4)")

        return 1.0 # some fake output, just so the node can be executed

Requested hidden input data of type "EXTRA_DATA" is passed to custom nodes
@IARI IARI requested a review from comfyanonymous as a code owner May 14, 2024 14:49
@RandomGitUser321
Copy link

How safe is something like this? Sounds like a good way for bad actors to potentially inject malicious exploits into workflows.

@IARI
Copy link
Author

IARI commented May 14, 2024

@RandomGitUser321 thats a good point.
I was also thinking that maybe this could be exploitet, here's my assesment:

clientside

  • What this PRs code does:
    add js data which will be stringified and attachted to the prompt.
  • How you can already hack it right now:
    As far as I can tell, nothing prevents you from importing and overwriting any of the already existing js api code.
    particularly your extension could easily import api.js and override the api.queuePrompt method.
    For example ComfyUi Manager does this in these places with:

serverside

  • What this PRs code does:
    It offers any custom node potentially access to all of the extra_data, where before it was only possible to access specific fields (namely the prompt, extra_pnginfo and unique_id)

  • Situation right now:
    I think there is no way to directly access other than the already defined extra_data data from a custom node so far.

possible exploit

It is conceivable that an extension A provides sensitive data with extra_data.
Another malicious extension could access extra_data from A with this PR.

Is there a point in trying to 'fix' something?

I could try that the extra data that extension A provides can only be read by nodes that also come from A.
But then again that's nearly impossible to accomplish, because there is no security with the client side.
Any extension can always overwrite methods from the api and thereby probably already intercept everything that ComfyUi and any other installed extension is doing clientside...
I don't really see a point fixing a small hole in the window when there's no roof on top.

Am I missing something critical, like any real vulnerability that isn't already there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants