-
Notifications
You must be signed in to change notification settings - Fork 94
poc #623
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
base: main
Are you sure you want to change the base?
poc #623
Conversation
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 introduces new configuration and prompt builder classes. However, a high-severity Prompt Injection vulnerability has been identified in packages/genui/lib/src/model/prompt_builder2.dart due to a flawed prompt construction pattern that discards user input and relies on an attacker-controllable system prompt. Additionally, the PR contains critical issues such as compile-time errors in prompt_builder2.dart (undefined type, method overloading), UnimplementedError in prompt_builder1.dart, and an empty pull request description, which violates contributing guidelines.
|
|
||
| String prompt(String userMessage); | ||
|
|
||
| GenUiPrompt prompt(UserMessage userMessage); |
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.
| String prompt(String userMessage); | ||
|
|
||
| GenUiPrompt prompt(UserMessage userMessage); |
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.
Dart does not support method overloading based on parameter types. The GenUiPromptBuilder abstract class declares two methods named prompt with different signatures, which will result in a compile-time error. You need to either rename one of them or consolidate their functionality into a single method.
| String prompt(String userMessage); | |
| GenUiPrompt prompt(UserMessage userMessage); | |
| GenUiPrompt prompt(UserMessage userMessage); |
| _CustomGenUiConfiguration(this._prompt, this._catalog); | ||
|
|
||
| @override | ||
| String prompt(UserMessage userMessage) => _prompt; |
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.
This prompt method in _CustomGenUiConfiguration is highly susceptible to Prompt Injection. It completely ignores the userMessage parameter, returning _prompt directly, which can be controlled by an attacker if systemPrompt is untrusted. This also creates a type mismatch, as the GenUiPromptBuilder interface (line 24) expects a GenUiPrompt return type, but the current implementation returns a String, leading to a compile-time error.
| String prompt(UserMessage userMessage) => _prompt; | |
| String prompt(UserMessage userMessage) => '${_prompt}\n\n${userMessage.toString()}'; |
| String prompt() => throw UnimplementedError(); | ||
|
|
||
| @override | ||
| ToolSet toolSet() => throw UnimplementedError(); |
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 prompt() and toolSet() methods in _BasicGenUiConfiguration currently throw UnimplementedError. If _BasicGenUiConfiguration is meant to be a concrete implementation, these methods should provide actual logic or sensible default values rather than throwing runtime errors. This could lead to unexpected crashes if this configuration is used.
@override
String prompt() => _prompt;
@override
ToolSet toolSet() => ToolSet();
No description provided.