Tarea 3838 - Añadidos pipes y ganchos a SendMail y comprobaciones de emails en copia en NewMail

[daniel89fg]

Se han añadido diferentes tipos de Pipes y ganchos para SendMail, así como comprobar y añadir emails en copia o copia oculta a NewMail cuando lo que hay configurado y lo que se manda es diferente.

¿Cómo has probado los cambios?

Toda modificación debe haber sido mínimamente probada. Marca o describe las pruebas que has realizado:

• He revisado mi código antes de enviarlo.
• He probado que funciona correctamente en mi PC.
• He probado que funciona correctamente con una base de datos vacía.
• He ejecutado los tests unitarios.

[Copilot]

Pull request overview

This pull request adds extensibility hooks (pipes) and improves email handling in the SendMail functionality. The main changes include adding multiple pipe/hook insertion points in the view template, validating and adding configured CC/BCC emails automatically in NewMail, and refactoring the SendMail controller to use consistent naming and validation patterns.

• Adds pipe/hook system to SendMail controller and template for extensibility
• Implements automatic addition of configured CC/BCC emails during send operation
• Refactors form field names and email validation logic

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File	Description
Core/View/SendMail.html.twig	Adds multiple pipe insertion points, updates form field names from generic to prefixed (email-to, email-subject, etc.), improves CC/BCC visibility logic based on pre-filled values
Core/Lib/Email/NewMail.php	Adds logic to automatically include configured CC/BCC emails from settings when sending, with validation
Core/Controller/SendMail.php	Adds ExtensionsTrait, implements pipe calls throughout, refactors setEmailAddress() to setEmail() with improved validation, adds email validation for all recipients

Comments suppressed due to low confidence (1)

Core/Controller/SendMail.php:391

• The setAttachment() method uses the user-controlled fileName request parameter directly when building the attachment path, allowing directory traversal to read arbitrary files from the server filesystem into outgoing emails. An attacker with access to this controller can craft a fileName value containing ../ segments to attach sensitive files (e.g. configuration or credential files) and exfiltrate them via email. Restrict fileName to a safe pattern (e.g. generated IDs or basenames), normalize and validate the resolved path against the intended attachments directory, and reject any value that attempts to escape it.

        $fileName = $this->request->queryOrInput('fileName', '');
        Tools::folderCheckOrCreate(NewMail::ATTACHMENTS_TMP_PATH);
        $this->newMail->addAttachment(FS_FOLDER . '/' . NewMail::ATTACHMENTS_TMP_PATH . $fileName, $fileName);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Spelling inconsistency: estableceos should be establecemos. The typo is in the comment.

Suggested change

	// estableceos el email de origen
	// establecemos el email de origen

[Copilot]

Inconsistent email parsing logic. The code uses str_replace(' ', '', $emailCC) which removes all spaces before splitting emails by comma. However, the send() method at line 348 uses NewMail::splitEmails() which only trims spaces around each email. This inconsistency could lead to different behavior. Consider using NewMail::splitEmails($emailCC) instead for consistency.

[Copilot]

Inconsistent email parsing logic. The code uses str_replace(' ', '', $emailBCC) which removes all spaces before splitting emails by comma. However, the send() method at line 366 uses NewMail::splitEmails() which only trims spaces around each email. This inconsistency could lead to different behavior. Consider using NewMail::splitEmails($emailBCC) instead for consistency.

[Copilot]

Inconsistent email parsing logic. The code uses str_replace(' ', '', $emailTo) which removes all spaces before splitting emails by comma. However, the send() method at line 330 uses NewMail::splitEmails() which only trims spaces around each email. This inconsistency could lead to different behavior. Consider using NewMail::splitEmails($emailTo) instead for consistency.

[Copilot]

Method name mismatch: The code calls getCcAddresses() (camelCase) but the method is defined as getCCAddresses() (uppercase) in NewMail.php (line 288). This will cause a fatal error. Use getCCAddresses() consistently.

Suggested change

	{% set emailsCC = fsc.newMail.getCcAddresses() is empty ? '' : fsc.newMail.getCCAddresses() | join(', ') ~ ', ' %}
	{% set emailsCC = fsc.newMail.getCCAddresses() is empty ? '' : fsc.newMail.getCCAddresses() | join(', ') ~ ', ' %}

[Copilot]

Method name mismatch: The code calls getBccAddresses() (camelCase) but the method is defined as getBCCAddresses() (uppercase) in NewMail.php (line 275). This will cause a fatal error. Use getBCCAddresses() consistently.

Suggested change

	{% set emailsCC = fsc.newMail.getCcAddresses() is empty ? '' : fsc.newMail.getCCAddresses() | join(', ') ~ ', ' %}
	{% set emailsBCC = fsc.newMail.getBccAddresses() is empty ? '' : fsc.newMail.getBccAddresses() | join(', ') ~ ', ' %}
	{% set emailsCC = fsc.newMail.getCCAddresses() is empty ? '' : fsc.newMail.getCCAddresses() | join(', ') ~ ', ' %}
	{% set emailsBCC = fsc.newMail.getBCCAddresses() is empty ? '' : fsc.newMail.getBCCAddresses() | join(', ') ~ ', ' %}

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.