Skip to content

extend to email address list #1363

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.

Sign up for GitHub

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

Merged
iceljc merged 1 commit into from
Jun 11, 2026
+17 −9
Merged

extend to email address list #1363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Plugins/BotSharp.Plugin.EmailHandler/Functions/HandleEmailSenderFn.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public HandleEmailSenderFn(
public async Task<bool> Execute(RoleDialogModel message)
{
var args = JsonSerializer.Deserialize<LlmContextIn>(message.FunctionArgs, _options.JsonSerializerOptions);
var recipient = args?.ToAddress;
var recipients = args?.ToAddresses?.Where(x => !string.IsNullOrWhiteSpace(x)) ?? [];
var body = args?.Content;
var subject = args?.Subject;
var isNeedAttachments = args?.IsNeedAttachemnts ?? false;
Comment on lines +32 to 35

@qodo-code-review qodo-code-review Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Recipients not validated empty 📘 Rule violation ☼ Reliability

HandleEmailSenderFn.Execute defaults recipients to an empty collection when args/ToAddresses
is null and then proceeds without validating that at least one recipient exists, so invalid boundary
inputs aren’t rejected early. This can mask client/LLM input errors (e.g., missing or mis-shaped
to_address_list) and lead to unpredictable downstream behavior or a late SMTP send failure instead
of a clear validation error.
Agent Prompt 
## Issue description
`HandleEmailSenderFn.Execute` currently accepts missing/empty boundary inputs by defaulting `recipients` to an empty collection and continuing execution, which can result in attempting to send an email with no recipients and failing late (SMTP/Mime creation) with a generic failure. Update the handler to validate required inputs at the boundary (at minimum: recipients; optionally: subject and body) and fail fast with a clear, predictable validation error/result returned to the caller, per compliance requirements.

## Issue Context
- `recipients` is computed with a null-coalescing default (e.g., `args?.ToAddresses ?? []`), so null/missing `to_address_list` is silently treated as “no recipients”.
- The PR changed input from `to_address` to `to_address_list`; the implementation filters null/whitespace and adds recipients in a loop, but never checks that any were actually added before calling the SMTP send.
- When the LLM omits or mis-shapes `to_address_list`, this becomes a runtime send attempt failure rather than a clear validation error.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.EmailHandler/Functions/HandleEmailSenderFn.cs[31-62]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand All @@ -39,7 +39,12 @@ public async Task<bool> Execute(RoleDialogModel message)
{
var mailMessage = new MimeMessage();
mailMessage.From.Add(new MailboxAddress(_emailSettings.Name, _emailSettings.EmailAddress));
mailMessage.To.Add(new MailboxAddress("", recipient));

foreach (var recipient in recipients)
{
mailMessage.To.Add(new MailboxAddress("", recipient));
}

mailMessage.Subject = subject;
bodyBuilder.TextBody = body;

Expand All @@ -53,7 +58,7 @@ public async Task<bool> Execute(RoleDialogModel message)
var response = await SendEmailBySMTP(mailMessage);
message.Content = response;

_logger.LogWarning($"Email successfully send over to {recipient}. Email Subject: {subject} [{response}]");
_logger.LogWarning($"Email successfully send over to {string.Join(",", recipients)}. Email Subject: {subject} [{response}]");

@qodo-code-review qodo-code-review Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Pii in interpolated log 📘 Rule violation ⛨ Security

The success log uses string interpolation and includes recipient email addresses and subject, which
can leak sensitive/PII into operational logs and is not structured for queryable context. It also
logs a success path at Warning level, which is not an appropriate severity.
Agent Prompt 
## Issue description
A success log message is written via string interpolation and includes recipient email addresses and the email subject. This can expose PII/sensitive data and is not structured logging; additionally, `LogWarning` is used for a successful operation.

## Issue Context
The code logs `string.Join(",", recipients)` and `subject` in a warning-level message.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.EmailHandler/Functions/HandleEmailSenderFn.cs[61-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

return true;
}
catch (Exception ex)
Expand Down
4 changes: 2 additions & 2 deletions src/Plugins/BotSharp.Plugin.EmailHandler/LlmContexts/LlmContextIn.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ namespace BotSharp.Plugin.EmailHandler.LlmContexts;

public class LlmContextIn
{
[JsonPropertyName("to_address")]
public string? ToAddress { get; set; }
[JsonPropertyName("to_address_list")]
public IEnumerable<string>? ToAddresses { get; set; }

[JsonPropertyName("email_content")]
public string? Content { get; set; }
Expand Down
11 changes: 7 additions & 4 deletions ...agents/6745151e-6d46-4a02-8de4-1c4f21c7da95/functions/util-email-handle_email_sender.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
"parameters": {
"type": "object",
"properties": {
"to_address": {
"type": "string",
"description": "The email address to which the email should be sent to. It needs to be a valid email address in the correct string format."
"to_address_list": {
"type": "array",
"description": "The email addresses to which the email should be sent. Each item needs to be a valid and unique email address in the correct string format.",
"items": {
"type": "string"
}
},
"email_content": {
"type": "string",
Expand All @@ -21,6 +24,6 @@
"description": "If the user request to send email with attachemnt(s) or file(s), then this value should be True. Otherwise, this value should be False."
}
},
"required": [ "to_address", "email_content", "subject", "is_need_attachments" ]
"required": [ "to_address_list", "email_content", "subject", "is_need_attachments" ]
}
}
Loading