Copilot commented on code in PR #3922:
URL: https://github.com/apache/hertzbeat/pull/3922#discussion_r2637779772
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/controller/ChatController.java:
##########
@@ -148,8 +149,21 @@ public ResponseEntity<Message<ChatConversation>>
getConversation(
@DeleteMapping(path = "/conversations/{conversationId}")
@Operation(summary = "Delete conversation", description = "Delete a
specific conversation and all its messages")
public ResponseEntity<Message<Void>> deleteConversation(
- @Parameter(description = "Conversation ID", example = "2345678")
@PathVariable("conversationId") Long conversationId) {
+ @Parameter(description = "Conversation ID", example = "2345678")
@PathVariable("conversationId") Long conversationId) {
conversationService.deleteConversation(conversationId);
return ResponseEntity.ok(Message.success());
}
+
+ /**
+ * Save data submitted by secure form
+ * @param securityData security data
+ * @return save result
+ */
+ @PostMapping(path = "/security")
+ @Operation(summary = "save security data", description = "Save security
data")
+ public ResponseEntity<Message<Boolean>> commitSecurityData(@Valid
@RequestBody SecurityData securityData) {
+ return
ResponseEntity.ok(Message.success(conversationService.saveSecurityData(securityData)));
+ }
Review Comment:
Missing input validation. The SecurityData object should be validated for
null conversationId and empty/null securityData before processing. Add
validation annotations like @NotNull on the conversationId field and @NotBlank
on securityData field in the SecurityData class, or add explicit validation in
the controller method.
##########
hertzbeat-common/src/main/java/org/apache/hertzbeat/common/entity/ai/ChatConversation.java:
##########
@@ -81,4 +82,6 @@ public class ChatConversation {
@OneToMany
@JoinColumn(name = "conversation_id")
private List<ChatMessage> messages;
+
+ private String securityData;
Review Comment:
The securityData field is added to ChatConversation entity without database
migration scripts or schema updates. Ensure that the database schema is updated
appropriately with proper migration scripts (e.g., Flyway or Liquibase) to add
this column, and verify that existing conversations can handle null values for
this field.
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
+ this.showSecurityFormModal = false;
Review Comment:
The modal is closed immediately after submitting (line 684), even before the
async saveSecurityData operation completes. This creates a race condition where
the modal closes regardless of whether the save operation succeeds or fails.
The modal should only close after a successful save, and should remain open if
there's an error so the user can retry.
```suggestion
.subscribe({
next: (message: any) => {
if (message.code === 0) {
const lastMessage = this.messages[this.messages.length - 1];
lastMessage.securityForm.complete = true;
const tmpMessage = this.newMessage;
this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
this.sendMessage();
this.newMessage = tmpMessage;
// Close the modal only after a successful save
this.showSecurityFormModal = false;
} else {
console.log('Error saving security data:');
// Keep the modal open so the user can correct and retry
}
},
error: (err: any) => {
console.log('Error saving security data:', err);
// Keep the modal open on error so the user can retry
}
});
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
Review Comment:
Missing error handler for the saveSecurityData observable. If the HTTP
request fails due to network issues or server errors, there's no error callback
to handle it. This could leave the modal in an inconsistent state. Add an error
handler in the subscribe method to handle failures gracefully.
```suggestion
.subscribe(
(message: any) => {
if (message.code === 0) {
const lastMessage = this.messages[this.messages.length - 1];
lastMessage.securityForm.complete = true;
const tmpMessage = this.newMessage;
this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
this.sendMessage();
this.newMessage = tmpMessage;
} else {
console.log('Error saving security data:');
}
},
(error: any) => {
console.error('Error saving security data:', error);
}
);
```
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java:
##########
@@ -92,9 +94,12 @@ public Flux<String> streamChat(ChatRequestContext context) {
log.info("Starting streaming chat for conversation: {}",
context.getConversationId());
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("conversationId", context.getConversationId());
+
log.info(SystemPromptTemplate.builder().resource(systemResource).build().createMessage(metadata).getText());
return chatClient.prompt()
.messages(messages)
-
.system(SystemPromptTemplate.builder().resource(systemResource).build().getTemplate())
+
.system(SystemPromptTemplate.builder().resource(systemResource).build().create(metadata).getContents())
Review Comment:
This change from getTemplate() to create(metadata).getContents() appears to
use an API that may not exist or may not be the correct method. The typical
Spring AI SystemPromptTemplate API uses createMessage(Map) or render(Map).
Verify that this method chain is correct for the version of Spring AI being
used.
```suggestion
.system(SystemPromptTemplate.builder().resource(systemResource).build().createMessage(metadata))
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
Review Comment:
Debug console.log statement should be removed before merging to production.
Error details should be logged properly or displayed to the user through the
message service.
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle security form cancel
+ */
+ onSecurityFormCancel(): void {
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle open security form
+ *
+ * @param securityForm
+ */
+ openSecurityForm(securityForm: SecurityForm): void {
+ this.securityParamDefine =
JSON.parse(securityForm.param).privateParams.map((i: any) => {
+ this.securityParams[i.field] = {
+ // Parameter type 0: number 1: string 2: encrypted string 3: json
string mapped by map
+ type: i.type === 'number' ? 0 : i.type === 'text' || i.type ===
'string' ? 1 : i.type === 'json' ? 3 : 2,
+ field: i.field,
+ paramValue: null
+ };
+ i.name = i.name[this.i18nSvc.defaultLang];
+ return i;
+ });
+
+ console.log('this.securityParamDefine:', this.securityParamDefine);
Review Comment:
Debug console.log statement should be removed before merging to production.
This exposes potentially sensitive parameter definitions in browser console
logs.
```suggestion
```
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java:
##########
@@ -92,9 +94,12 @@ public Flux<String> streamChat(ChatRequestContext context) {
log.info("Starting streaming chat for conversation: {}",
context.getConversationId());
+ Map<String, Object> metadata = new HashMap<>();
+ metadata.put("conversationId", context.getConversationId());
+
log.info(SystemPromptTemplate.builder().resource(systemResource).build().createMessage(metadata).getText());
Review Comment:
This debug log statement on line 99 will log the entire system message which
could be very large and may contain sensitive information about the system's
capabilities. Consider removing this debug log or moving it behind a more
restrictive log level check before production deployment.
```suggestion
if (log.isDebugEnabled()) {
String systemPromptText = SystemPromptTemplate.builder()
.resource(systemResource)
.build()
.createMessage(metadata)
.getText();
log.debug("Generated system prompt for conversation {}
(length={} characters)",
context.getConversationId(),
systemPromptText != null ? systemPromptText.length()
: 0);
}
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle security form cancel
+ */
+ onSecurityFormCancel(): void {
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle open security form
+ *
+ * @param securityForm
+ */
+ openSecurityForm(securityForm: SecurityForm): void {
+ this.securityParamDefine =
JSON.parse(securityForm.param).privateParams.map((i: any) => {
+ this.securityParams[i.field] = {
+ // Parameter type 0: number 1: string 2: encrypted string 3: json
string mapped by map
+ type: i.type === 'number' ? 0 : i.type === 'text' || i.type ===
'string' ? 1 : i.type === 'json' ? 3 : 2,
+ field: i.field,
+ paramValue: null
+ };
+ i.name = i.name[this.i18nSvc.defaultLang];
+ return i;
+ });
Review Comment:
Missing JSON parse error handling. If securityForm.param contains invalid
JSON, JSON.parse() will throw an uncaught exception that crashes the component.
Add try-catch error handling to gracefully handle malformed JSON and provide
user feedback.
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/MonitorToolsImpl.java:
##########
@@ -194,44 +210,45 @@ public String queryMonitors(
@Override
@Tool(name = "add_monitor", description = """
- HertzBeat: Add a new monitoring target to HertzBeat with
comprehensive configuration.
- This tool dynamically handles different parameter requirements for
each monitor type.
-
- This tool creates monitors with proper app-specific parameters.
-
- *********
- VERY IMPORTANT:
- ALWAYS use get_monitor_additional_params to check the additional
required parameters for the chosen type before adding a monitor or even
mentioning it.
- Use list_monitor_types tool to see available monitor type names to
use here in the app parameter.
- Use the information obtained from this to query user for
parameters.
- If the User has not given any parameters, ask them to provide the
necessary parameters, until all the necessary parameters are provided.
- **********
-
- Examples of natural language requests this tool handles:
- - "Monitor website example.com with HTTPS on port 443"
- - "Add MySQL monitoring for database server at 192.168.1.10 with
user admin"
- - "Monitor Linux server health on host server.company.com via SSH"
- - "Set up Redis monitoring on localhost port 6379 with password"
-
- PARAMETER MAPPING: Use the 'params' parameter to pass all
monitor-specific configuration.
- The params should be a JSON string containing key-value pairs for
the monitor type.
- Use get_monitor_additional_params tool to see what parameters are
required for each monitor type.
-
- PARAMS EXAMPLES:
- - Website: {"host":"example.com", "port":"443",
"uri":"/api/health", "ssl":"true", "method":"GET"}
- - Linux: {"host":"192.168.1.10", "port":"22", "username":"root",
"password":"xxx"}
- - MySQL: {"host":"db.server.com", "port":"3306",
"username":"admin", "password":"xxx", "database":"mydb"}
- - Redis: {"host":"redis.server.com", "port":"6379",
"password":"xxx"}
- """)
+ HertzBeat: Add a new monitoring target to HertzBeat with comprehensive
configuration.
+ This tool dynamically handles different parameter requirements for
each monitor type.
+
+ This tool creates monitors with proper app-specific parameters.
+
+ *********
+ VERY IMPORTANT:
+ ALWAYS use get_monitor_additional_params to check the additional
required parameters for the chosen type before adding a monitor or even
mentioning it.
+ Use list_monitor_types tool to see available monitor type names to use
here in the app parameter.
+ Use the information obtained from this to query user for parameters.
+ If the User has not given any parameters, ask them to provide the
necessary parameters, until all the necessary parameters are provided.
+ **********
+
+ Examples of natural language requests this tool handles:
+ - "Monitor website example.com with HTTPS on port 443"
+ - "Add MySQL monitoring for database server at 192.168.1.10 with user
admin"
+ - "Monitor Linux server health on host server.company.com via SSH"
+ - "Set up Redis monitoring on localhost port 6379 with password"
+
+ PARAMETER MAPPING: Use the 'params' parameter to pass all
monitor-specific configuration.
+ The params should be a JSON string containing key-value pairs for the
monitor type.
+ Use get_monitor_additional_params tool to see what parameters are
required for each monitor type.
+
+ PARAMS EXAMPLES:
+ - Website: {"host":"example.com", "port":"443", "uri":"/api/health",
"ssl":"true", "method":"GET"}
+ - Linux: {"host":"192.168.1.10", "port":"22", "username":"root",
"password":"xxx"}
+ - MySQL: {"host":"db.server.com", "port":"3306", "username":"admin",
"password":"xxx", "database":"mydb"}
+ - Redis: {"host":"redis.server.com", "port":"6379", "password":"xxx"}
+ """)
public String addMonitor(
- @ToolParam(description = "Monitor name (required)", required =
true) String name,
- @ToolParam(description = "Monitor type: website, mysql,
postgresql, redis, linux, windows, etc.", required = true) String app,
- @ToolParam(description = "Collection interval in seconds (default:
600)", required = false) Integer intervals,
- @ToolParam(description = "Monitor-specific parameters as JSON
string. "
- + "Use get_monitor_additional_params to see required
fields. "
- + "Example: {\"host\":\"192.168.1.1\", \"port\":\"22\",
\"username\":\"root\"}",
- required = true) String params,
- @ToolParam(description = "Monitor description (optional)",
required = false) String description) {
+ @ToolParam(description = "The id for current conversation", required =
true) Long conversationId,
+ @ToolParam(description = "Monitor name (required)", required = true)
String name,
+ @ToolParam(description = "Monitor type: website, mysql, postgresql,
redis, linux, windows, etc.", required = true) String app,
+ @ToolParam(description = "Collection interval in seconds (default:
600)", required = false) Integer intervals,
+ @ToolParam(description = "Monitor-specific parameters as JSON string. "
+ + "Use get_monitor_additional_params to see required fields. "
+ + "Example: {\"host\":\"192.168.1.1\", \"port\":\"22\",
\"username\":\"root\"}",
+ required = true) String params,
+ @ToolParam(description = "Monitor description (optional)", required =
false) String description) {
Review Comment:
The addMonitor method now requires a conversationId parameter but there's no
validation to ensure it's not null. If conversationId is null, the code will
fail when trying to query the conversation. Add validation to check that
conversationId is provided and valid before proceeding with monitor creation.
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/service/impl/ChatClientProviderServiceImpl.java:
##########
@@ -54,13 +56,13 @@ public class ChatClientProviderServiceImpl implements
ChatClientProviderService
private final ApplicationContext applicationContext;
private final GeneralConfigDao generalConfigDao;
-
+
@Autowired
private ToolCallbackProvider toolCallbackProvider;
-
+
private boolean isConfigured = false;
- @Value("classpath:/prompt/system-message.st")
+ @Value("classpath:/prompt/system-message-improve.st")
Review Comment:
The system prompt template file path was changed from 'system-message.st' to
'system-message-improve.st', but based on the directory listing, only
'system-message.st' exists. This indicates either the file should be renamed to
'system-message-improve.st' or this line should revert to use
'system-message.st'. The current diff shows changes to system-message.st,
suggesting the @Value annotation should remain as 'system-message.st'.
```suggestion
@Value("classpath:/prompt/system-message.st")
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
Review Comment:
There's no validation to ensure that securityParams contains valid data
before sending. If a required field is empty or invalid, the backend should
validate it, but the frontend should also validate before submission to provide
immediate user feedback. Consider adding form validation before calling
saveSecurityData.
```suggestion
// Basic client-side validation to ensure all security parameters have
values
const paramsArray = this.securityParams ?
Object.values(this.securityParams) : [];
const hasInvalidParam = paramsArray.some((param: any) => {
const value = param && param.paramValue;
return value === null || value === undefined || value === '';
});
if (hasInvalidParam) {
// Provide immediate feedback to the user and prevent submission
alert('Please fill in all required security fields before
submitting.');
return;
}
this.aiChatService
.saveSecurityData({
securityData: JSON.stringify(paramsArray),
```
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/MonitorToolsImpl.java:
##########
@@ -251,32 +268,43 @@ public String addMonitor(
if (intervals == null || intervals < 10) {
intervals = 600;
}
-
// Parse params to extract host and port for instance
List<Param> paramList = parseParams(params);
+
+ // Query and add sensitive parameters
+ Optional<ChatConversation> chatConversation =
conversationDao.findById(conversationId);
+ if (chatConversation.isPresent() &&
StringUtils.isNotEmpty(chatConversation.get().getSecurityData())) {
+ List<Param> securityParams = JsonUtil.fromJson(
+
AesUtil.aesDecode(chatConversation.get().getSecurityData()),
+ new TypeReference<List<Param>>() {
+ });
+ if (CollectionUtils.isNotEmpty(securityParams)) {
+ paramList.addAll(securityParams);
Review Comment:
Missing JSON deserialization error handling. If the decrypted securityData
contains invalid JSON, JsonUtil.fromJson() will fail. Add error handling to
catch deserialization exceptions and provide meaningful error messages.
```suggestion
String decryptedSecurityData =
AesUtil.aesDecode(chatConversation.get().getSecurityData());
try {
List<Param> securityParams = JsonUtil.fromJson(
decryptedSecurityData,
new TypeReference<List<Param>>() {
});
if (CollectionUtils.isNotEmpty(securityParams)) {
paramList.addAll(securityParams);
}
} catch (Exception ex) {
log.error("Failed to deserialize securityData for
conversationId {}: {}", conversationId, ex.getMessage(), ex);
return "Error: Failed to parse stored security
parameters for this conversation. "
+ "Please clear or reconfigure the security data and
try again. Details: " + ex.getMessage();
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.html:
##########
@@ -229,3 +238,25 @@ <h3 class="welcome-title">{{ 'ai.chat.welcome.title' |
i18n }}</h3>
</form>
</div>
</nz-modal>
+
+<!-- Security Form Modal -->
+<nz-modal
+ [(nzVisible)]="showSecurityFormModal"
+ [nzTitle]="'ai.chat.security.form.title' | i18n"
+ (nzOnCancel)="onSecurityFormCancel()"
+ (nzOnOk)="onSecurityFormSubmit()"
+ nzMaskClosable="false"
+>
+ <div *nzModalContent class="-inner-content">
+ <form nz-form #form="ngForm">
+ <ng-container *ngFor="let paramDefine of securityParamDefine">
+ <nz-form-item>
+ <nz-form-label nzSpan="7" [nzRequired]="paramDefine.required"
[nzFor]="paramDefine.field">{{ paramDefine.name }}</nz-form-label>
+ <nz-form-control nzSpan="8" [nzErrorTip]="'validation.required' |
i18n">
+ <app-form-field [item]="paramDefine" [name]="paramDefine.field"
[(ngModel)]="securityParams[paramDefine.field].paramValue" />
+ </nz-form-control>
+ </nz-form-item>
+ </ng-container>
+ </form>
+ </div>
+</nz-modal>
Review Comment:
The security form modal lacks proper form validation. The template
references form="ngForm" but there's no validation logic to check if required
fields are filled before allowing submission. The submit button should be
disabled when the form is invalid, similar to how Angular forms typically work
with [disabled]="!form.valid".
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
Review Comment:
The error handling here only logs to console without showing any user
feedback. When security data fails to save, users should be notified through a
message service. Consider adding this.message.error() to display an error
message to the user, similar to other error handling in this component.
```suggestion
.subscribe(
(message: any) => {
if (message.code === 0) {
const lastMessage = this.messages[this.messages.length - 1];
lastMessage.securityForm.complete = true;
const tmpMessage = this.newMessage;
this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
this.sendMessage();
this.newMessage = tmpMessage;
} else {
console.error('Error saving security data:', message);
this.message.error('Failed to save security data.');
}
},
(error: any) => {
console.error('Error saving security data:', error);
this.message.error('Failed to save security data.');
}
);
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
Review Comment:
There's no validation to check if messages array is empty before accessing
the last message. If the messages array is empty, this will throw an error. Add
a check to ensure the array has elements before accessing
messages[messages.length - 1].
```suggestion
const lastMessage = this.messages.length > 0 ?
this.messages[this.messages.length - 1] : null;
if (lastMessage && lastMessage.securityForm) {
lastMessage.securityForm.complete = true;
}
```
##########
hertzbeat-ai/src/main/java/org/apache/hertzbeat/ai/tools/impl/MonitorToolsImpl.java:
##########
@@ -251,32 +268,43 @@ public String addMonitor(
if (intervals == null || intervals < 10) {
intervals = 600;
}
-
// Parse params to extract host and port for instance
List<Param> paramList = parseParams(params);
+
+ // Query and add sensitive parameters
+ Optional<ChatConversation> chatConversation =
conversationDao.findById(conversationId);
+ if (chatConversation.isPresent() &&
StringUtils.isNotEmpty(chatConversation.get().getSecurityData())) {
+ List<Param> securityParams = JsonUtil.fromJson(
+
AesUtil.aesDecode(chatConversation.get().getSecurityData()),
+ new TypeReference<List<Param>>() {
+ });
+ if (CollectionUtils.isNotEmpty(securityParams)) {
+ paramList.addAll(securityParams);
Review Comment:
No error handling for AES decryption failure. If the securityData is
corrupted or was encrypted with a different key, AesUtil.aesDecode() could
throw an exception that's not caught here. Add try-catch to handle decryption
failures gracefully and provide appropriate error messages.
```suggestion
try {
List<Param> securityParams = JsonUtil.fromJson(
AesUtil.aesDecode(chatConversation.get().getSecurityData()),
new TypeReference<List<Param>>() {
});
if (CollectionUtils.isNotEmpty(securityParams)) {
paramList.addAll(securityParams);
}
} catch (Exception e) {
log.error("Failed to decrypt or parse security data for
conversationId {}", conversationId, e);
return "Error: Failed to decrypt stored secure
parameters. Please reconfigure secure parameters for this conversation.";
```
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -253,6 +266,26 @@ export class ChatComponent implements OnInit, OnDestroy {
});
}
+ /**
+ * calculate if the security form should be shown for the given message
content
+ *
+ * @param content current message content
+ * @param next next message content
+ */
+ calculateShowSecurityForm(content: string, next: string): SecurityForm {
+ const completeMessage: String[] = ['表单填写完成', '表單已完成', 'Formulário
concluído', 'フォームが完了しました', 'Form completed'];
Review Comment:
This array of hardcoded translated strings is brittle and error-prone for
detecting form completion. If any translation changes or new languages are
added, this will break. Consider using a different mechanism, such as checking
a specific flag in the message metadata or using a consistent message key that
can be detected programmatically.
##########
web-app/src/app/shared/components/ai-chat/chat.component.ts:
##########
@@ -618,4 +658,57 @@ export class ChatComponent implements OnInit, OnDestroy {
this.aiProviderConfig.model = selectedProvider.defaultModel;
}
}
+
+ /**
+ * handle security form submit
+ *
+ */
+ onSecurityFormSubmit(): void {
+ this.aiChatService
+ .saveSecurityData({
+ securityData: JSON.stringify(Object.values(this.securityParams)),
+ conversationId: this.currentConversation?.id
+ })
+ .subscribe((message: any) => {
+ if (message.code === 0) {
+ const lastMessage = this.messages[this.messages.length - 1];
+ lastMessage.securityForm.complete = true;
+ const tmpMessage = this.newMessage;
+ this.newMessage =
this.i18nSvc.fanyi('ai.chat.security.form.default.callback');
+ this.sendMessage();
+ this.newMessage = tmpMessage;
+ } else {
+ console.log('Error saving security data:');
+ }
+ });
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle security form cancel
+ */
+ onSecurityFormCancel(): void {
+ this.showSecurityFormModal = false;
+ }
+
+ /**
+ * handle open security form
+ *
+ * @param securityForm
+ */
+ openSecurityForm(securityForm: SecurityForm): void {
+ this.securityParamDefine =
JSON.parse(securityForm.param).privateParams.map((i: any) => {
+ this.securityParams[i.field] = {
+ // Parameter type 0: number 1: string 2: encrypted string 3: json
string mapped by map
+ type: i.type === 'number' ? 0 : i.type === 'text' || i.type ===
'string' ? 1 : i.type === 'json' ? 3 : 2,
+ field: i.field,
+ paramValue: null
+ };
+ i.name = i.name[this.i18nSvc.defaultLang];
Review Comment:
The code assumes i.name[this.i18nSvc.defaultLang] will always exist, but if
the AI response doesn't include a translation for the current language, this
will result in undefined. Consider adding a fallback to a default language
(e.g., 'en-US') or the original name property to prevent displaying undefined
to users.
```suggestion
i.name = i.name[this.i18nSvc.defaultLang] || i.name['en-US'] || i.name;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
