From d7fd5423de70cfe010a85f35f3b4d1b032ae4561 Mon Sep 17 00:00:00 2001
From: Chirag Aggarwal <chiragaggarwal5k@gmail.com>
Date: Wed, 28 Oct 2026 15:45:56 +0000
Subject: [PATCH] fix(storage): let callers refuse endpoint overrides in a
 tenant's DSN

The url and insecure parameters replace the host a DSN names. An operator sets them on purpose, but a DSN holding tenant values could carry them through a spliced bucket and send every request to an address the tenant chose (appwrite-labs/cloud#6857). getDevice(trusted: false) refuses them, and the local device, so the host the caller checked is the one reached.
---
 src/Executor/StorageFactory.php            | 13 +++++-
 tests/unit/Executor/StorageFactoryTest.php | 46 ++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/Executor/StorageFactory.php b/src/Executor/StorageFactory.php
index b902cd6..5ced81e 100644
--- a/src/Executor/StorageFactory.php
+++ b/src/Executor/StorageFactory.php
@@ -26,15 +26,20 @@ class StorageFactory
      * @param string $root Root path for storage
      * @param ?string $connection DSN connection string. An empty or null connection means no object storage is configured for this deployment, and a `file` or `local` scheme asks for the local device outright. Both yield the local device rooted at $root.
      * @param (ClientInterface&StreamingClientInterface)|null $client HTTP client for the remote devices, defaulting to the one utopia-php/storage builds
+     * @param bool $trusted False when the connection holds a tenant's values: its host is then final, so the `url` and `insecure` parameters, which replace it, are refused, and so is the local device
      *
-     * @throws InvalidArgumentException When the connection string cannot be parsed or names a scheme with no device
+     * @throws InvalidArgumentException When the connection string cannot be parsed, names a scheme with no device, or is untrusted and asks for what only an operator may
      */
-    public static function getDevice(string $root, #[\SensitiveParameter] ?string $connection = '', (ClientInterface&StreamingClientInterface)|null $client = null): Device
+    public static function getDevice(string $root, #[\SensitiveParameter] ?string $connection = '', (ClientInterface&StreamingClientInterface)|null $client = null, bool $trusted = true): Device
     {
         $connection ??= '';
         $localSchemes = ['file', DeviceType::Local->value];
 
         if ($connection === '' || \in_array(\strtolower((string) \strstr($connection, '://', true)), $localSchemes, true)) {
+            if (!$trusted) {
+                throw new InvalidArgumentException('Untrusted storage DSN must name a remote device');
+            }
+
             return new Local($root);
         }
 
@@ -44,6 +49,10 @@ public static function getDevice(string $root, #[\SensitiveParameter] ?string $c
             throw new InvalidArgumentException('Unable to parse storage DSN');
         }
 
+        if (!$trusted && ($dsn->getParam('url') !== '' || $dsn->getParam('insecure') !== '')) {
+            throw new InvalidArgumentException('Untrusted storage DSN cannot replace its endpoint');
+        }
+
         $scheme = $dsn->getScheme();
         $deviceType = DeviceType::tryFrom($scheme)
             ?? throw new InvalidArgumentException(sprintf("Storage DSN scheme '%s' has no device", $scheme));
diff --git a/tests/unit/Executor/StorageFactoryTest.php b/tests/unit/Executor/StorageFactoryTest.php
index 47a57e6..6fa6fed 100644
--- a/tests/unit/Executor/StorageFactoryTest.php
+++ b/tests/unit/Executor/StorageFactoryTest.php
@@ -200,6 +200,52 @@ public function testUnusableConnectionIsRefused(string $connection): void
         StorageFactory::getDevice('/storage/builds/app-test', $connection);
     }
 
+    /**
+     * @return \Iterator<string, array{?string}>
+     */
+    public static function untrustedOverrides(): \Iterator
+    {
+        yield 'url' => ['s3://key:secret@minio.example.com/b?region=us-east-1&url=' . \urlencode('http://169.254.169.254/latest')];
+        yield 'insecure' => ['s3://key:secret@minio.example.com/b?region=us-east-1&insecure=true'];
+        yield 'url spliced in through the bucket' => ['s3://key:secret@minio.example.com/b?url=http://169.254.169.254/latest&insecure=true'];
+        yield 'local scheme' => ['local://localhost'];
+        yield 'file scheme' => ['file:///etc'];
+        yield 'empty' => [''];
+        yield 'null' => [null];
+    }
+
+    #[DataProvider('untrustedOverrides')]
+    public function testUntrustedConnectionCannotReplaceItsEndpoint(?string $connection): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        StorageFactory::getDevice('/', $connection, trusted: false);
+    }
+
+    public function testUntrustedConnectionReachesItsOwnHost(): void
+    {
+        $client = new class () implements ClientInterface, StreamingClientInterface {
+            public string $url = '';
+
+            public function sendRequest(RequestInterface $request): ResponseInterface
+            {
+                $this->url = (string) $request->getUri();
+
+                return new Response(200)->withHeader('ETag', '"etag"');
+            }
+
+            public function stream(RequestInterface $request, callable $sink): ResponseInterface
+            {
+                return $this->sendRequest($request);
+            }
+        };
+
+        StorageFactory::getDevice('/', 's3://key:secret@minio.example.com:9000/b?region=us-east-1', $client, trusted: false)
+            ->write(self::OBJECT_KEY, new Stream('wal'), 'application/octet-stream');
+
+        $this->assertSame('https://minio.example.com:9000/b/' . self::OBJECT_KEY, $client->url);
+    }
+
     public function testRefusalDoesNotPrintTheConnection(): void
     {
         $ignoreArgs = \ini_set('zend.exception_ignore_args', '0');