Skip to content

fix: prevent email recipient exposure in BCC#11300

Open
tanujbhaud wants to merge 4 commits intoappwrite:mainfrom
tanujbhaud:fix/email-privacy-bcc-all-providers
Open

fix: prevent email recipient exposure in BCC#11300
tanujbhaud wants to merge 4 commits intoappwrite:mainfrom
tanujbhaud:fix/email-privacy-bcc-all-providers

Conversation

@tanujbhaud
Copy link

This PR addresses a privacy issue where email recipients were visible to each other when they should have been BCC'd or sent individually. Fixes #8261.

…pwrite#8261)

Previously, only SMTP provider moved recipients to BCC. Other providers
(Mailgun, SendGrid, Resend) placed all recipients in the TO field,
exposing everyone's email addresses to each other — a GDPR concern.

Now, when sending to multiple recipients, all are moved to BCC
regardless of the email provider.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 11, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69c0693 and d6dac4d.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Workers/Messaging.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Appwrite/Platform/Workers/Messaging.php

📝 Walkthrough

Walkthrough

sendExternalMessage now determines maxMessagesPerRequest per provider: if the provider's type is MESSAGE_TYPE_EMAIL it is forced to 1, otherwise it uses the adapter's getMaxMessagesPerRequest(). This computed value is used to chunk the provider's identifiers into batches when dispatching external messages, replacing the previous unconditional use of the adapter's getMaxMessagesPerRequest() for batching.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: preventing email recipient exposure in BCC, which directly addresses the privacy issue fixed in this PR.
Description check ✅ Passed The description is relevant and explains the purpose of the PR, addressing privacy issues with email recipients being visible to each other.
Linked Issues check ✅ Passed The code change enforces sending emails one at a time (maxMessagesPerRequest=1 for email) to prevent recipient exposure, directly addressing issue #8261's requirement that recipients not see other targets' addresses.
Out of Scope Changes check ✅ Passed All changes in the PR are directly related to fixing the email privacy issue by modifying how email batches are processed per provider type.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/Appwrite/Platform/Workers/Messaging.php`:
- Around line 599-612: When batching multiple recipients in Messaging.php the
current logic sets $to = [] if $fromEmail is not provided, causing downstream
adapters (which expect at least one TO recipient via getTo()) to fail; change
the handling in the block that moves recipients to BCC so that if $fromEmail is
empty you keep the first recipient in $to and move the remaining recipients to
$bcc (i.e., set $to = [array_shift($to)] and push the rest into $bcc), ensuring
$to is never empty while still hiding other recipients; reference the $to, $bcc
and $fromEmail variables and the provider['options']['fromEmail'] usage when
applying the change.

@github-actions
Copy link

github-actions bot commented Feb 11, 2026

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libcrypto3 3.5.4-r0 CVE-2025-15467 CRITICAL
libcrypto3 3.5.4-r0 CVE-2025-69419 HIGH
libcrypto3 3.5.4-r0 CVE-2025-69421 HIGH
libecpg 17.7-r0 CVE-2026-2004 HIGH
libecpg 17.7-r0 CVE-2026-2005 HIGH
libecpg 17.7-r0 CVE-2026-2006 HIGH
libecpg 17.7-r0 CVE-2026-2007 HIGH
libecpg-dev 17.7-r0 CVE-2026-2004 HIGH
libecpg-dev 17.7-r0 CVE-2026-2005 HIGH
libecpg-dev 17.7-r0 CVE-2026-2006 HIGH
libecpg-dev 17.7-r0 CVE-2026-2007 HIGH
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng 1.6.51-r0 CVE-2026-22695 HIGH
libpng 1.6.51-r0 CVE-2026-22801 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2026-22695 HIGH
libpng-dev 1.6.51-r0 CVE-2026-22801 HIGH
libpq 17.7-r0 CVE-2026-2004 HIGH
libpq 17.7-r0 CVE-2026-2005 HIGH
libpq 17.7-r0 CVE-2026-2006 HIGH
libpq 17.7-r0 CVE-2026-2007 HIGH
libpq-dev 17.7-r0 CVE-2026-2004 HIGH
libpq-dev 17.7-r0 CVE-2026-2005 HIGH
libpq-dev 17.7-r0 CVE-2026-2006 HIGH
libpq-dev 17.7-r0 CVE-2026-2007 HIGH
libssl3 3.5.4-r0 CVE-2025-15467 CRITICAL
libssl3 3.5.4-r0 CVE-2025-69419 HIGH
libssl3 3.5.4-r0 CVE-2025-69421 HIGH
openssl 3.5.4-r0 CVE-2025-15467 CRITICAL
openssl 3.5.4-r0 CVE-2025-69419 HIGH
openssl 3.5.4-r0 CVE-2025-69421 HIGH
openssl-dev 3.5.4-r0 CVE-2025-15467 CRITICAL
openssl-dev 3.5.4-r0 CVE-2025-69419 HIGH
openssl-dev 3.5.4-r0 CVE-2025-69421 HIGH
postgresql17-dev 17.7-r0 CVE-2026-2004 HIGH
postgresql17-dev 17.7-r0 CVE-2026-2005 HIGH
postgresql17-dev 17.7-r0 CVE-2026-2006 HIGH
postgresql17-dev 17.7-r0 CVE-2026-2007 HIGH
py3-urllib3 1.26.20-r0 CVE-2026-21441 HIGH
py3-urllib3-pyc 1.26.20-r0 CVE-2026-21441 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/Appwrite/Platform/Workers/Messaging.php`:
- Around line 599-618: The current logic in Messaging.php moves all original
recipients to BCC and sets $to = [$fromEmail] when $fromEmail is present, which
causes the sender to receive every batch email; instead, always pick the first
original recipient as the TO and move the remaining $to entries to $bcc
regardless of $fromEmail. In the block handling multiple recipients (variables
$to, $bcc, $fromEmail), change the flow to array_shift $to into $first, push the
remaining recipients into $bcc as ['email' => $recipient], and set $to =
[$first]; do not set $to to $fromEmail or otherwise add $fromEmail into
$to/envelope so the sender is not an automatic delivery target. Ensure the
fallback branch is removed or unified so both code paths use this
first-recipient-as-TO pattern.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/Appwrite/Platform/Workers/Messaging.php`:
- Around line 599-606: The current logic in Messaging.php leaves the first
recipient in $to when multiple recipients are present, exposing that address;
change to send per-recipient (batch size = 1) instead of a single message with
BCC: iterate over the original recipient list and, for each recipient, set $to
to that single recipient and clear $bcc before calling the existing send routine
(the method that ultimately sends the message in this class), or build
per-recipient message objects and call the send method for each; ensure $to and
$bcc are only populated for the single target so no recipient ever sees another
recipient’s address.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc52a45 and 0d17c53.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Workers/Messaging.php

@tanujbhaud tanujbhaud force-pushed the fix/email-privacy-bcc-all-providers branch from 0d17c53 to 69c0693 Compare February 23, 2026 20:11
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/Appwrite/Platform/Workers/Messaging.php (2)

254-263: Consider provider rate limits under concurrent single-recipient batching.

With $maxMessagesPerRequest = 1, a message sent to N email recipients spawns N concurrent Swoole coroutines (via the inner batch() at line 265), each making a separate API call to the provider. For large recipient lists this can produce large request bursts that hit provider-side rate limits (e.g., SendGrid's getMaxMessagesPerRequest() returns 1000, indicating its API is designed for bulk delivery; forcing individual calls bypasses that). Failures from rate limiting are caught and recorded as $deliveryErrors, so they're not silently lost, but there's no retry/backoff in this path.

Worth confirming the expected recipient-count scale and whether a concurrency cap or send-rate limiter should be applied per provider.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/Platform/Workers/Messaging.php` around lines 254 - 263, The code
forces $maxMessagesPerRequest = 1 for MESSAGE_TYPE_EMAIL which causes N
concurrent batch() coroutines for N recipients and can burst provider rate
limits; update the logic around getMaxMessagesPerRequest()/MESSAGE_TYPE_EMAIL
and the batching that builds $batches so that email sends either respect the
provider's bulk capacity or are subject to a per-provider concurrency/send-rate
limiter before invoking batch(): use getMaxMessagesPerRequest() as the upper
bound, or implement a concurrency cap/rate limiter keyed by the provider (e.g.,
wrap calls from batch() with a provider-specific semaphore/tokens or throttle)
and ensure delivery errors ($deliveryErrors) still record failures but
backoff/retry or queue excess batches instead of launching unlimited coroutines.

254-262: Privacy fix approach is sound; optional cleanup for dead getMaxMessagesPerRequest() call on email adapters.

The core fix — forcing $maxMessagesPerRequest = 1 for all MESSAGE_TYPE_EMAIL providers — correctly ensures every email is dispatched with a single recipient in TO, fully resolving the exposure issue without any BCC manipulation. All past review concerns (empty TO, sender copy, first-recipient visibility) are addressed by this approach.

As a minor cleanup, since getMaxMessagesPerRequest() is immediately discarded for email providers, the call can be deferred:

♻️ Avoid calling `getMaxMessagesPerRequest()` for email adapters
-                $maxMessagesPerRequest = $adapter->getMaxMessagesPerRequest();
-
-                if ($provider->getAttribute('type') === MESSAGE_TYPE_EMAIL) {
-                    $maxMessagesPerRequest = 1;
-                }
+                if ($provider->getAttribute('type') === MESSAGE_TYPE_EMAIL) {
+                    $maxMessagesPerRequest = 1;
+                } else {
+                    $maxMessagesPerRequest = $adapter->getMaxMessagesPerRequest();
+                }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Appwrite/Platform/Workers/Messaging.php` around lines 254 - 262, The code
currently calls $adapter->getMaxMessagesPerRequest() unconditionally and then
overrides $maxMessagesPerRequest when $provider->getAttribute('type') ===
MESSAGE_TYPE_EMAIL; change the order so you first check the provider type and
only call $adapter->getMaxMessagesPerRequest() when the provider is not an email
(MESSAGE_TYPE_EMAIL), setting $maxMessagesPerRequest = 1 for emails otherwise;
update the logic around $maxMessagesPerRequest (used in array_chunk with
array_keys($identifiersForProvider)) so the call is deferred and avoids the
unnecessary getMaxMessagesPerRequest() invocation for email adapters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Appwrite/Platform/Workers/Messaging.php`:
- Around line 254-263: The code forces $maxMessagesPerRequest = 1 for
MESSAGE_TYPE_EMAIL which causes N concurrent batch() coroutines for N recipients
and can burst provider rate limits; update the logic around
getMaxMessagesPerRequest()/MESSAGE_TYPE_EMAIL and the batching that builds
$batches so that email sends either respect the provider's bulk capacity or are
subject to a per-provider concurrency/send-rate limiter before invoking batch():
use getMaxMessagesPerRequest() as the upper bound, or implement a concurrency
cap/rate limiter keyed by the provider (e.g., wrap calls from batch() with a
provider-specific semaphore/tokens or throttle) and ensure delivery errors
($deliveryErrors) still record failures but backoff/retry or queue excess
batches instead of launching unlimited coroutines.
- Around line 254-262: The code currently calls
$adapter->getMaxMessagesPerRequest() unconditionally and then overrides
$maxMessagesPerRequest when $provider->getAttribute('type') ===
MESSAGE_TYPE_EMAIL; change the order so you first check the provider type and
only call $adapter->getMaxMessagesPerRequest() when the provider is not an email
(MESSAGE_TYPE_EMAIL), setting $maxMessagesPerRequest = 1 for emails otherwise;
update the logic around $maxMessagesPerRequest (used in array_chunk with
array_keys($identifiersForProvider)) so the call is deferred and avoids the
unnecessary getMaxMessagesPerRequest() invocation for email adapters.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d17c53 and 69c0693.

📒 Files selected for processing (1)
  • src/Appwrite/Platform/Workers/Messaging.php

@tanujbhaud tanujbhaud force-pushed the fix/email-privacy-bcc-all-providers branch from 69c0693 to d6dac4d Compare February 23, 2026 20:24
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.

🐛 Bug Report: Email from other targets/users are sent to all targets by default

1 participant