feat(client): optional TLS for the MongoDB connection#40
Conversation
Add a `$tls` constructor flag (default false, BC-preserving) that opens the Swoole socket with SWOOLE_SSL and enables encryption. Peer verification is disabled (ssl_verify_peer=false, ssl_allow_self_signed=true) because the backend is reached through a TLS-terminating proxy whose hostname is not on the backend's own certificate. Self-hosted plaintext connections are unaffected. Needed so Appwrite Cloud can reach a dedicated MongoDB backend (documentsdb) through the edge TCP proxy, which mandates TLS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Client constructor now accepts ChangesTLS Support for MongoDB Client Connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds optional TLS to the Swoole-backed MongoDB
Confidence Score: 5/5The change is small, backward-compatible, and correctly wires TLS into the Swoole socket without touching the existing plaintext path. The implementation is straightforward: one bitwise flag addition and one conditional array_merge. Existing callers are unaffected. No hardcoded verification policy is introduced; that responsibility is correctly delegated to the caller. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "refactor(client): let caller own TLS ver..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Client.php`:
- Around line 145-146: The current boolean $tls parameter (in
Client::__construct / Client::connect) only toggles TLS and leads to setting
ssl_verify_peer = false, which disables certificate validation; change the API
to accept structured TLS options (e.g., replace or augment bool $tls with an
array or additional params such as $tls = true, ?string $caFile = null, ?string
$peerName = null, bool $verifyPeer = true) so verification remains enabled by
default, allow callers to supply a CA bundle/path and expected peer name for
hostname validation, and keep the existing "disable verification" behavior only
as an explicit opt-in (verifyPeer = false) used as an escape hatch; update all
locations that read $tls or set ssl_verify_peer (including the blocks around the
current $tls param and the code referenced at 199-205) to use the new options
and to set ssl_verify_peer based on verifyPeer rather than unconditionally
turning it off.
- Around line 181-188: The code sets SWOOLE_SSL when $tls is true without
checking if that constant exists; update the logic in Client.php around $flags,
$tls, and the client construction (the block manipulating $flags and creating
CoroutineClient / SwooleClient) to first check defined('SWOOLE_SSL') before
OR-ing SWOOLE_SSL, and if $tls is true but SWOOLE_SSL is not defined throw a
clear descriptive exception (or otherwise surface a configuration error)
indicating "Swoole must be built with SSL to enable TLS", so consumers get a
clear failure instead of undefined constant behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c577cac-98e1-404e-95fc-aa3f83cc13a9
📒 Files selected for processing (1)
src/Client.php
| $flags = SWOOLE_SOCK_TCP | SWOOLE_KEEP; | ||
| if ($tls) { | ||
| $flags |= SWOOLE_SSL; | ||
| } | ||
|
|
||
| $this->client = $useCoroutine | ||
| ? new CoroutineClient(SWOOLE_SOCK_TCP | SWOOLE_KEEP) | ||
| : new SwooleClient(SWOOLE_SOCK_TCP | SWOOLE_KEEP); | ||
| ? new CoroutineClient($flags) | ||
| : new SwooleClient($flags); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files that may declare/runtime-document Swoole requirements =="
fd -HI 'composer\.json|README\.md|Dockerfile|docker-compose.*|\.tool-versions|\.github/workflows/.*'
echo
echo "== References to Swoole/OpenSwoole/SSL support in the repo =="
rg -n -C2 'swoole|openswoole|SWOOLE_SSL|openssl' .Repository: utopia-php/mongo
Length of output: 5794
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json: Swoole/extension requirements =="
rg -n --no-heading 'swoole|ext-swoole|openssl|ssl|tls|SWOOLE_SSL' composer.json || true
echo
echo "== README.md: TLS/SSL/Swoole notes =="
rg -n --no-heading 'tls|ssl|openssl|SWOOLE_SSL|swoole' README.md || true
echo
echo "== src: TLS option usage around $tls in Client.php =="
rg -n -C3 'function __construct|\\$tls\\b|tls\\b|SWOOLE_SSL|CoroutineClient|SwooleClient' src/Client.phpRepository: utopia-php/mongo
Length of output: 421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json: Swoole/extension requirements =="
rg -n --no-heading 'ext-swoole|swoole\/|swoole|openssl|ssl|tls|SWOOLE_SSL' composer.json || true
echo
echo "== README.md: TLS/SSL/Swoole notes =="
rg -n --no-heading 'tls|ssl|openssl|SWOOLE_SSL|swoole' README.md || true
echo
echo "== src/Client.php: $tls handling around lines ~181 =="
sed -n '150,230p' src/Client.php
echo
echo "== src/Client.php: where $tls is defined =="
rg -n '(\$tls\b|function __construct|__construct\(|public function|private function)' src/Client.phpRepository: utopia-php/mongo
Length of output: 517
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json: Swoole/extension requirements =="
rg -n --no-heading 'ext-swoole|swoole/|openssl|ssl|tls|SWOOLE_SSL' composer.json || true
echo
echo "== README.md: TLS/SSL/Swoole notes =="
rg -n --no-heading 'tls|ssl|openssl|SWOOLE_SSL|swoole' README.md || true
echo
echo "== src/Client.php: TLS option usage around lines ~181 =="
sed -n '150,230p' src/Client.php
echo
echo "== src/Client.php: where TLS ($tls) is defined/used =="
rg -n '(\$tls\b|__construct\(|new CoroutineClient|new SwooleClient|SWOOLE_SSL)' src/Client.phpRepository: utopia-php/mongo
Length of output: 3233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== composer.json: check for ext-swoole/swoole/tls requirements =="
cat composer.json
echo
echo "== src/Client.php: all tls occurrences =="
rg -n '\btls\b|SWOOLE_SSL|ssl_verify_peer|ssl_allow_self_signed' src/Client.php
echo
echo "== src/Client.php: constructor signature/docs =="
rg -n 'function __construct|__construct\(|public function __construct' src/Client.php
sed -n '1,140p' src/Client.php
echo
echo "== repo-wide: tls/ssl/swoole ssl build requirements mentions =="
rg -n --no-heading '\b(ext-swoole|ext_swoole|swoole|SWOOLE_SSL|ssl_verify_peer|ssl_allow_self_signed|tls)\b' README.md src .github workflows Dockerfile docker-compose.yml composer.json || trueRepository: utopia-php/mongo
Length of output: 6483
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,210p' src/Client.phpRepository: utopia-php/mongo
Length of output: 2895
Harden $tls handling when Swoole is not built with SSL
When $tls is true, src/Client.php unconditionally uses SWOOLE_SSL ($flags |= SWOOLE_SSL), but the repo doesn’t declare or document an SSL-enabled Swoole/OpenSwoole build requirement (only swoole/ide-helper is present in require-dev, and README.md has no TLS/SSL requirement). Add a guard (e.g., defined('SWOOLE_SSL') with a descriptive exception) or explicitly enforce/document the “Swoole built with SSL” requirement.
$flags = SWOOLE_SOCK_TCP | SWOOLE_KEEP;
if ($tls) {
$flags |= SWOOLE_SSL;
}
$this->client = $useCoroutine
? new CoroutineClient($flags)
: new SwooleClient($flags);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Client.php` around lines 181 - 188, The code sets SWOOLE_SSL when $tls is
true without checking if that constant exists; update the logic in Client.php
around $flags, $tls, and the client construction (the block manipulating $flags
and creating CoroutineClient / SwooleClient) to first check
defined('SWOOLE_SSL') before OR-ing SWOOLE_SSL, and if $tls is true but
SWOOLE_SSL is not defined throw a clear descriptive exception (or otherwise
surface a configuration error) indicating "Swoole must be built with SSL to
enable TLS", so consumers get a clear failure instead of undefined constant
behavior.
Replace the hardcoded ssl_verify_peer=false with a $tlsOptions array merged into the socket options when TLS is enabled. The driver provides the TLS mechanism; the consumer decides verification — verify against the system CA in production, or relax it where the endpoint presents an untrusted certificate (e.g. a staging Let's Encrypt staging cert). Avoids baking a verification downgrade into the driver for all environments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Adds optional TLS to the Mongo
Client: abool $tls = falseflag opens the Swoole socket withSWOOLE_SSL, and aarray $tlsOptions = []lets the caller set the verification policy (ssl_verify_peer,ssl_cafile,ssl_host_name,ssl_allow_self_signed, …). Defaults off — self-hosted plaintext connections are unaffected.Why
Appwrite Cloud's dedicated MongoDB backends (documentsdb) are reached through the edge TCP proxy, which requires TLS. The driver had no TLS support. The driver intentionally does not hardcode a verification policy — the consumer verifies against the system CA in production, and may relax verification only where the endpoint presents an untrusted cert (e.g. staging uses a Let's Encrypt staging certificate that isn't publicly trusted).
Notes
$tlsdefaults false; existing 6-arg callers unchanged.🤖 Generated with Claude Code
Summary by CodeRabbit