-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
[Frontend] Require flag for loading text and image embeds #27204
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
Conversation
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
…is not passed Signed-off-by: DarkLight1337 <[email protected]>
|
Documentation preview: https://vllm--27204.org.readthedocs.build/en/27204/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses security vulnerabilities GHSA-mrw7-hf4f-83pf and GHSA-pmqf-x6x8-p7qw by introducing --enable-prompt-embeds and --enable-mm-embeds flags. These flags gate the functionality of loading user-provided text and multimodal embeddings, which can be a security risk. The changes are well-implemented across the configuration, core logic, and entrypoints, with checks to ensure the flags are respected. The documentation has been updated with clear warnings, and new tests have been added to verify the behavior. The implementation appears correct and robust. I have not found any issues in this pull request.
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
|
cc @christian-pinto |
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
| `prompt_embeds` key. | ||
| WARNING: The vLLM engine may crash if incorrect shape of embeddings is passed. | ||
| Only enable this flag for trusted users!""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should a shape check be added to the renderer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shape depends on the embedding size of each model so it requires a lot more effort to perform shape validation outside of the model class.
…ct#27204) Signed-off-by: DarkLight1337 <[email protected]> Co-authored-by: DarkLight1337 <[email protected]>
…ct#27204) Signed-off-by: DarkLight1337 <[email protected]> Co-authored-by: DarkLight1337 <[email protected]> Signed-off-by: Alberto Perdomo <[email protected]>
…ct#27204) Signed-off-by: DarkLight1337 <[email protected]> Co-authored-by: DarkLight1337 <[email protected]>
…ct#27204) Signed-off-by: DarkLight1337 <[email protected]> Co-authored-by: DarkLight1337 <[email protected]> Signed-off-by: 0xrushi <[email protected]>
…ct#27204) Signed-off-by: DarkLight1337 <[email protected]> Co-authored-by: DarkLight1337 <[email protected]> Signed-off-by: 0xrushi <[email protected]>
https://github.com/vllm-project/vllm/security/advisories/GHSA-mrw7-hf4f-83pf
https://github.com/vllm-project/vllm/security/advisories/GHSA-pmqf-x6x8-p7qw
Issue #26928