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

Escape HTML in user metadata description before appending to card #13241

Conversation

zixaphir
Copy link
Contributor

@zixaphir zixaphir commented Sep 14, 2023

Description

Currently the model description is appended to a card on the document as-is. This allows for the user to inadvertently add HTML to the model description shown on-card and have it be appended onto the DOM if show descriptions on card is enabled. On a single-user system this presents a minimal security hazard, but on a shared system, any user could insert malicious code into the card metadata via the metadata editor.

This also can cause issues where HTML in the description results in unexpected behavior, like the model card or other cards ceasing to display. For instance, replacing the model description with <div> causes the model to vanish on refresh, while setting it to <div><div> causes other models to cease appearing. In a real-world case, zixaphir/Stable-Diffusion-Webui-Civitai-Helper#10 is caused by the text <title>_<trigger word> appearing in the model description. While I can easily fix this on my end by removing the offending text, it will result in an unexpected state for the user where important informal text is removed, like information about other models used (IE, "used detailer model with <lora:detailer:0.6>)"

Screenshots/videos:

Checklist:

Currently the description is appended to the document as-is. This
allows for the user to inadvertently add HTML to the model
description shown on-card and have it be appended onto the DOM if
`show descriptions on card` is enabled. On a single-user system
this presents a minimal security hazard, but on a shared system,
any user could insert malicious code into the card metadata via
the metadata editor.
@AUTOMATIC1111
Copy link
Owner

The idea is to let users write descriptions using HTML - it's meant to work this way.

@zixaphir
Copy link
Contributor Author

While I very much do want to argue against this as the default use-case for use-case sake, I suppose it isn't my place. But I will argue from UI perspective.

I do still believe this creates unexpected behavior, especially for non-technical users where the overlap between lora syntax and html syntax means that an average user would be expected to not just understand why writing <lora:my_lora:1> in their description causes issues but also be expected to read and write escape characters in the case that they do want to include the <> characters.

I do hope you reconsider this.

@AUTOMATIC1111
Copy link
Owner

I imagine there are a lot of descriptions written by users already, and some would be using HTML, so changing the behavior here would be detrimental for them.

A way to go would be to support .txt files with descriptions that do get escaped, and .html files that do not get escaped. But it's still a disruptive change for people who would have to rename their txt to html.

If we do this, UI description editor would also have to be updated to let user choose if he wants to input the description as plaintext or HTML.

@zixaphir
Copy link
Contributor Author

zixaphir commented Oct 1, 2023

Well, given that, I suppose I'll close this to not clutter your issue tracker. I don't personally have any interest in HTML in the description for extra networks, so the legwork to expand that in any meaningful way would be better left to somebody who does.

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