Skip to content

Optional Amazon SES mailer + trim AWS SDK + docs - #5

Merged
psantus merged 2 commits into
mainfrom
feat/ses-mail
Sep 8, 2026
Merged

Optional Amazon SES mailer + trim AWS SDK + docs#5
psantus merged 2 commits into
mainfrom
feat/ses-mail

Conversation

@psantus

@psantus psantus commented Sep 8, 2026

Copy link
Copy Markdown
Owner

SPIP mail() doesn't work on Lambda. Adds an optional SES mailer and trims the bundled AWS SDK.

  • ses_mail plugin (spip/plugins/ses_mail): overrides inc_envoyer_mail to send via SES. Optional & self-disabling — falls back to native mail() when SES_FROM is unset, so it's inert until you set ses_from_email in tfvars (which also attaches the ses:SendEmail IAM policy + injects SES_FROM/SES_REGION).
  • shrink-vendor.sh: KEEP_SERVICES trimmed to dsql|dynamodb|s3|ssm|sts + ses/email; dropped bedrock-*/translate/cloudfront/lambda residue → leaner image.
  • Docs: new bilingual docs/{fr,en}/email-ses.md; build.md KEEP_SERVICES note updated; README task tables linked.

Paul Santus added 2 commits September 8, 2026 15:14
…services

- spip/plugins/ses_mail: overrides inc_envoyer_mail to send via SES; OPTIONAL and
  self-disabling (falls back to native mail() when SES_FROM is unset).
- shrink-vendor.sh: KEEP_SERVICES trimmed to dsql|dynamodb|s3|ssm|sts + ses(email);
  dropped bedrock-*/translate/cloudfront/lambda residue.
…DME links

- Dockerfile: COPY ses_mail into plugins-dist (always loaded, inert without SES_FROM).
- docs/{fr,en}/email-ses.md: how the optional SES mailer works + how to enable (verify
  identity, set ses_from_email, sandbox caveat).
- build.md: KEEP_SERVICES note updated to the trimmed set.

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Summary

This PR adds optional Amazon SES email support and trims the AWS SDK bundle, which are valuable additions for the Lambda environment. The implementation is well-documented with bilingual documentation.

Critical Issues Found

Security Vulnerabilities (2)

  • Email header injection vulnerabilities in the native mail fallback function that could allow attackers to inject malicious headers via CRLF sequences

Performance Issue (1)

  • Potential performance degradation from recreating the SES client on every email send

Recommendation

The core functionality is sound, but the security vulnerabilities must be addressed before merge. Please apply the suggested fixes for header injection protection.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

$texte = is_array($corps) ? ($corps['texte'] ?? '') : (string) $corps;
$hdr = '';
if ($from) {
$hdr .= 'From: ' . $from . "\r\n";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Security Vulnerability: Email header injection risk. The $from parameter is used directly without validation in the native fallback. An attacker could inject malicious headers (e.g., additional recipients, BCC addresses) by passing newline characters in the From address. Validate and sanitize the $from parameter before using it in mail headers to prevent header injection attacks1.

Suggested change
$hdr .= 'From: ' . $from . "\r\n";
$hdr .= 'From: ' . str_replace(["\r", "\n"], '', $from) . "\r\n";

Footnotes

  1. CWE-93: Improper Neutralization of CRLF Sequences - https://cwe.mitre.org/data/definitions/93.html

Comment on lines +128 to +130
if ($headers) {
$hdr .= is_array($headers) ? implode("\r\n", $headers) : $headers;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Security Vulnerability: Email header injection risk via $headers parameter. An attacker could inject malicious email headers by manipulating the $headers parameter. Strip all CRLF sequences from headers to prevent header injection attacks1.

Suggested change
if ($headers) {
$hdr .= is_array($headers) ? implode("\r\n", $headers) : $headers;
}
if ($headers) {
$hdr .= is_array($headers) ? implode("\r\n", array_map(function($h) { return str_replace(["\r", "\n"], '', $h); }, $headers)) : str_replace(["\r", "\n"], '', $headers);
}

Footnotes

  1. CWE-93: Improper Neutralization of CRLF Sequences - https://cwe.mitre.org/data/definitions/93.html

Comment on lines +80 to +85
try {
require_once '/var/task/vendor/autoload.php';
$client = new \Aws\Ses\SesClient([
'version' => 'latest',
'region' => $ses_region,
]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Logic Error: Potential memory leak from repeated autoload. The require_once is inside a function that may be called multiple times during a request. Each call will evaluate the file path check. Move autoloader inclusion outside the function or cache the client instance to avoid repeated filesystem checks and potential performance degradation.

    static $client = null;

    try {
        if ($client === null) {
            require_once '/var/task/vendor/autoload.php';
            $client = new \Aws\Ses\SesClient([
                'version' => 'latest',
                'region'  => $ses_region,
            ]);
        }

@psantus
psantus merged commit d214c10 into main Sep 8, 2026
5 checks passed
@psantus
psantus deleted the feat/ses-mail branch September 8, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant