AUTOMATIC1111 / stable-diffusion-webui-extensions

Extension index for stable-diffusion-webui
483 stars 261 forks source link

[add][pr]sd-webui-decadetw-auto-prompt-llm #355

Closed xlinx closed 1 month ago

xlinx commented 1 month ago

Info

Checklist:

xlinx commented 1 month ago

the extension git URL: https://github.com/xlinx/sd-webui-decadetw-auto-prompt-llm readme0

light-and-ray commented 1 month ago

You can do this thing right in python, http requests are not something special for browsers

resultX = await fetch('http://localhost:1234/v1/chat/completions', {

You can rewrite it using python library

light-and-ray commented 1 month ago

Here is an example how to use API in python with no selenium: https://github.com/light-and-ray/sd-webui-replacer/blob/master/apiExample.py

xlinx commented 1 month ago
  1. okay, got u. i will try remove selenium lib, if need post fetch in python
  2. but the script function here is try to offer the JS basic codimg ability in prompting, like "for" "if" "else" "map" "localStorage set and get" "Using Browser IndexedDB" function
    %%{
    const currentTime = new Date().getHours();
    if (currentTime < 12) {
    localStorage.setItem("lasTimeCall", "Morning ");
    return  "Good Morning sd-prompt"; 
    } else if (currentTime < 18) {
    localStorage.setItem("lasTimeCall", "Afternoon ");
    return  "Good Afternoon sd-prompt"; 
    } else {
    localStorage.setItem("lasTimeCall", "Evening ");
    return  "Good Evening sd-prompt"; 
    }
    }%%
  3. according to the 2.(basic codimg ability) , may I keep use selenium ?
light-and-ray commented 1 month ago

I'm sure it also can be easily rewritten in python. Selenium is a library for full-featured web browser automatization, it's not a tool for these things. But take a note, I'm not a collaborator, I can't merge your pr. @w-e-w can

But I think selenium should be removed entirely. It's very big overhead

w-e-w commented 1 month ago

if you wish to provide some sort of JavaScript scripting functionality unless there is a very good reason that it should be processed on client side browser) allowing user submitted code to be run on server side is basically security security issue waiting to happen

I've already mentioned this in Discord because I want a second opinion but basically what I list the primary issues are with running JavaScript on server side is

  1. SSRF
  2. arbitrary file download to a fixed directory, this itself may not be catastrophic but it's certainly not good
  3. if there is somehow a vulnerability in Webdriver that allows escaping the sandbox it's basically Auburn Code Execution

adding server-side code execution extension is allowed if the user is fully aware of the potential issues and the future is put behind safeguards that needs to be mentally enabled

example the built-in custom code script I can only be used with --allow-code command line arg enabled example

these two extensions/scripts are meant for developer use, not for regularly end user


personally I really don't see the point of providing full scripting functionality for users in prompt box if someone really needs that sort of functionality then they might as well just use the API with API they have full control over everything and not just a the prompt

lite syntax that simplifies certain prompting repetitive tasks yes, I can get behind the idea but full scripting functionality no I myself have an extension that adds simple syntax they already exist several extensions that provides complex syntax for example https://github.com/ThereforeGames/unprompted https://github.com/adieyal/sd-dynamic-prompts I never used them so I'm not sure how useful they are but they are quite popular extensions

light-and-ray commented 1 month ago

On client side you can run js code on button click using _js argument

xlinx commented 1 month ago

@light-and-ray thx for ur time to check and pick the Selenium issue out. @w-e-w I have read the post seriously about 1hr to understand all the issue; I'm understand the basically security security issue now. I do ignore the people use this are running on server and need to be take care about the most normal users conditions. Its thank your to point it out with so much examples and knowledge.

  1. remove all the JS feature form my fhunction
  2. only keep LLM feature in this extension (https://github.com/xlinx/sd-webui-decadetw-auto-prompt-llm)
  3. thx detailed comments again, keep SD environment be safe.
w-e-w commented 1 month ago

I made some fixes to your implementation


that's also CRUCIAL information missing from your readme

your extension is extension that calls to EXTERNAL LLM server like lm studio or any other server that is compatible with OpenAI's client but you never actually told the user to actually install a external server

from a user's perspective they expect your extension to work without installing anything else unless you told them which you didn't

you only show a list of Suggestion software but you never actually told why you're why you're suggesting these software for all a average user knows you're just recommending me softness because you like them

you need to explicitly told the user to install LLM server along with configuration guide on how to configure your Setup to link with those server

yes by default the configuration is for lm studio but if they are not using you need to tell the user that they need to configure them


also is there a particular reason why you don't put your setup settings in .... well Settings tab?

xlinx commented 1 month ago

yea, I forget most people maybe first time meet LLM enviroment. [ADD] installtion in detail.

Installtion

w-e-w commented 1 month ago

some minor modification I added additional information to description

-    "url": "https://github.com/xlinx/sd-webui-decadetw-auto-prompt-llm",
+    "url": "https://github.com/xlinx/sd-webui-decadetw-auto-prompt-llm.git",
-    "description": "Auto Prompt with LLM",
+    "description": "Auto Prompt with LLM. Read README.md, additional installation steps required",
w-e-w commented 1 month ago

@xlinx confirmation, do you wish me to merge into the index index now?

xlinx commented 1 month ago

yes, please.