Conversation
There was a problem hiding this comment.
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.
| String prompt(String userMessage); | ||
|
|
||
| GenUiPrompt prompt(UserMessage userMessage); |
There was a problem hiding this comment.
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.
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.
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.