From 953b73af0d331b4761fad0c194d530a5da2c0726 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 15 Oct 2026 16:43:47 +1300 Subject: [PATCH] feat: confine source requests to a destinations policy Appwrite validates a migration source's endpoint before it connects, but the source then resolves the host again for every request and follows redirects anywhere, so a rebinding host or a redirect reaches the internal network. utopia-php/client already checks each connection and redirect hop against the address it actually reached through Destinations; this lets a caller hand a target that policy instead of re-implementing pinning. Target::setDestinations() applies to the target's own requests, and the Appwrite source passes it on to its SDK client through a thin subclass that adds it to the HTTP client the SDK builds. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 ++ src/Migration/Sources/Appwrite.php | 14 +- src/Migration/Sources/Appwrite/Client.php | 36 ++++++ src/Migration/Target.php | 18 ++- .../Migration/Unit/General/RequestTarget.php | 42 ++++++ .../Unit/General/TargetDestinationsTest.php | 121 ++++++++++++++++++ .../resources/destinations/server.php | 26 ++++ 7 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 src/Migration/Sources/Appwrite/Client.php create mode 100644 tests/Migration/Unit/General/RequestTarget.php create mode 100644 tests/Migration/Unit/General/TargetDestinationsTest.php create mode 100644 tests/Migration/resources/destinations/server.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b906e08..0c0ab1ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 3.2.0 + +### Added + +- `Target::setDestinations()` confines a target's requests to a `Utopia\Client\Destinations`. Each connection and + redirect hop is checked against the address it actually reached, so DNS cannot answer differently between a check + and the connection. Without it, requests connect anywhere, as before. +- The Appwrite source applies its destinations to its SDK client, `Sources\Appwrite\Client`, as well as to its own + requests. + ## 3.1.2 3.1.2 reverses the 3.0.0 notes below on `Aborted`, `markAborted()` and finalizing with `success()`. diff --git a/src/Migration/Sources/Appwrite.php b/src/Migration/Sources/Appwrite.php index a014bf86..de2ef3cd 100644 --- a/src/Migration/Sources/Appwrite.php +++ b/src/Migration/Sources/Appwrite.php @@ -3,7 +3,6 @@ namespace Utopia\Migration\Sources; use Appwrite\AppwriteException; -use Appwrite\Client; use Appwrite\Enums\ProjectAuthMethodId; use Appwrite\Enums\ProjectPolicyId; use Appwrite\Enums\ProjectProtocolId; @@ -19,6 +18,7 @@ use Appwrite\Services\Users; use Appwrite\Services\Webhooks; use Override; +use Utopia\Client\Destinations; use Utopia\Database\Database as UtopiaDatabase; use Utopia\Database\DateTime as UtopiaDateTime; use Utopia\Database\Document as UtopiaDocument; @@ -85,6 +85,7 @@ use Utopia\Migration\Resources\Storage\File; use Utopia\Migration\Resources\Templates\EmailTemplate; use Utopia\Migration\Source; +use Utopia\Migration\Sources\Appwrite\Client; use Utopia\Migration\Sources\Appwrite\Reader; use Utopia\Migration\Sources\Appwrite\Reader\API as APIReader; use Utopia\Migration\Sources\Appwrite\Reader\Database as DatabaseReader; @@ -139,7 +140,8 @@ public function __construct( protected ?UtopiaDatabase $dbForProject = null, protected array $queries = [], ) { - $this->client = (new Client()) + $this->client = new Client(); + $this->client ->setEndpoint($endpoint) ->setProject($projectId) ->setKey($key) @@ -187,6 +189,14 @@ public static function getName(): string return 'Appwrite'; } + #[Override] + public function setDestinations(Destinations $destinations): static + { + $this->client->setDestinations($destinations); + + return parent::setDestinations($destinations); + } + #[Override] public function run( array $resources, diff --git a/src/Migration/Sources/Appwrite/Client.php b/src/Migration/Sources/Appwrite/Client.php new file mode 100644 index 00000000..ca036ade --- /dev/null +++ b/src/Migration/Sources/Appwrite/Client.php @@ -0,0 +1,36 @@ +destinations = $destinations; + $this->resetHttpClient(); + + return $this; + } + + #[Override] + protected function createHttpClient(bool $followRedirects): HttpClient + { + $client = parent::createHttpClient($followRedirects); + + if ($this->destinations === null) { + return $client; + } + + return $client->withDestinations($this->destinations); + } +} diff --git a/src/Migration/Target.php b/src/Migration/Target.php index 5aa72e47..ea2da5b8 100644 --- a/src/Migration/Target.php +++ b/src/Migration/Target.php @@ -5,6 +5,7 @@ use Psr\Http\Client\ClientExceptionInterface; use Utopia\Client\Adapter\Curl\Client as CurlAdapter; use Utopia\Client\Client; +use Utopia\Client\Destinations; use Utopia\Psr7\ContentType; use Utopia\Psr7\Request\Factory as RequestFactory; @@ -56,6 +57,8 @@ abstract class Target private ?Client $client = null; + private ?Destinations $destinations = null; + protected string $rootResourceId = ''; protected string $rootResourceType = ''; @@ -157,6 +160,19 @@ protected function call( return $responseBody; } + /** + * Where this target's requests may connect. Each connection and redirect hop is + * checked against the address it actually reached, so DNS cannot answer differently + * between a check and the connection. Unset, requests may connect anywhere. + */ + public function setDestinations(Destinations $destinations): static + { + $this->destinations = $destinations; + $this->client = null; + + return $this; + } + /** * The HTTP client for {@see call()}, built once per target so connections are reused. */ @@ -166,7 +182,7 @@ protected function client(): Client \CURLOPT_TIMEOUT_MS => 0, \CURLOPT_LOW_SPEED_LIMIT => 1, \CURLOPT_LOW_SPEED_TIME => static::STALL_TIMEOUT, - ]))) + ]), $this->destinations)) ->withFollowRedirects() ->withConnectTimeout(static::CONNECT_TIMEOUT); } diff --git a/tests/Migration/Unit/General/RequestTarget.php b/tests/Migration/Unit/General/RequestTarget.php new file mode 100644 index 00000000..1f0b767f --- /dev/null +++ b/tests/Migration/Unit/General/RequestTarget.php @@ -0,0 +1,42 @@ +|string + * + * @throws \Exception + */ + public function fetch(string $url): array|string + { + return $this->call('GET', $url); + } +} diff --git a/tests/Migration/Unit/General/TargetDestinationsTest.php b/tests/Migration/Unit/General/TargetDestinationsTest.php new file mode 100644 index 00000000..2d2bd0de --- /dev/null +++ b/tests/Migration/Unit/General/TargetDestinationsTest.php @@ -0,0 +1,121 @@ +server)) { + \proc_terminate($this->server, 9); + \proc_close($this->server); + } + + $this->server = null; + } + + public function testTargetConnectsAnywhereByDefault(): void + { + $this->assertSame(['reached' => true], $this->target()->fetch($this->serve())); + } + + public function testTargetRefusesAnAddressItsDestinationsDoNotAllow(): void + { + $target = $this->target()->setDestinations(new PublicInternet()); + + try { + $target->fetch($this->serve()); + $this->fail('A loopback address must be refused under PublicInternet.'); + } catch (\Exception $error) { + $this->assertSame(Exception::CODE_INTERNAL, $error->getCode()); + $this->assertStringContainsString(self::REFUSED, $error->getMessage()); + } + } + + public function testTargetReachesARangeItsDestinationsAdmit(): void + { + $target = $this->target()->setDestinations(new PublicInternet(new IPRange('127.0.0.0/8'))); + + $this->assertSame(['reached' => true], $target->fetch($this->serve())); + } + + public function testTargetAppliesDestinationsSetAfterARequest(): void + { + $url = $this->serve(); + $target = $this->target(); + $target->fetch($url); + + $target->setDestinations(new PublicInternet()); + + $this->expectExceptionMessage(self::REFUSED); + $target->fetch($url); + } + + public function testSdkClientRefusesAnAddressItsDestinationsDoNotAllow(): void + { + $client = (new Client())->setDestinations(new PublicInternet()); + $client->setEndpoint($this->serve()); + + $this->expectException(AppwriteException::class); + $this->expectExceptionMessage(self::REFUSED); + $client->call(Client::METHOD_GET, '/health'); + } + + public function testSdkClientReachesARangeItsDestinationsAdmit(): void + { + $client = (new Client())->setDestinations(new PublicInternet(new IPRange('127.0.0.0/8'))); + $client->setEndpoint($this->serve()); + + $this->assertSame(['reached' => true], $client->call(Client::METHOD_GET, '/health')); + } + + public function testAppwriteSourcePassesItsDestinationsToTheSdkClient(): void + { + $source = new class ('project', $this->serve(), 'key', fn () => null) extends Appwrite { + public function sdk(): Client + { + return $this->client; + } + }; + + $this->assertSame(['reached' => true], $source->sdk()->call(Client::METHOD_GET, '/health')); + + $source->setDestinations(new PublicInternet()); + + $this->expectExceptionMessage(self::REFUSED); + $source->sdk()->call(Client::METHOD_GET, '/health'); + } + + private function target(): RequestTarget + { + return new RequestTarget(); + } + + private function serve(): string + { + $script = \dirname(__DIR__, 2).'/resources/destinations/server.php'; + $server = \proc_open([PHP_BINARY, $script], [1 => ['pipe', 'w']], $pipes); + $this->assertIsResource($server); + $this->server = $server; + + $port = \trim((string) \fgets($pipes[1])); + $this->assertMatchesRegularExpression('/^\d+$/', $port, 'The test server did not report its port.'); + + return "http://127.0.0.1:{$port}"; + } +} diff --git a/tests/Migration/resources/destinations/server.php b/tests/Migration/resources/destinations/server.php new file mode 100644 index 00000000..1fa6d3d3 --- /dev/null +++ b/tests/Migration/resources/destinations/server.php @@ -0,0 +1,26 @@ +