From a2373d673cbe1d5f9e75829bd0c3a4221d5ebcc1 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 24 Sep 2026 20:11:14 +1200 Subject: [PATCH] fix: keep the storage connection out of refusals getDevice() receives the connection string with its access and secret keys in it, and a refusal leaked it three ways: - its message forwarded utopia-php/dsn's, which in 0.2.1 and earlier quotes the whole DSN back when parse_url() rejects it; - the chained DSN exception carried that same message, and its DSN::__construct() frame, into anything that prints the exception; - every exception raised beneath getDevice() printed the connection argument in its trace, cut to 15 bytes by default, which is the scheme and the start of the credentials. Traces carry arguments unless zend.exception_ignore_args is on, and it is off by default and in appwrite/utopia-base, which ships no php.ini. The message matters most: the build path copies an exception's message into the build output it returns, so a malformed OPR_EXECUTOR_CONNECTION_STORAGE would put the operator's keys in build logs. The refusal now only says the DSN could not be parsed, and chains nothing, so what it prints no longer depends on which dsn version a consumer resolves (utopia-php/dsn#10 fixes the parser's message at the source). $connection is #[\SensitiveParameter], as utopia-php/storage already does for the secret key. Co-Authored-By: Claude Opus 5.5 --- src/Executor/StorageFactory.php | 6 +++--- tests/unit/Executor/StorageFactoryTest.php | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Executor/StorageFactory.php b/src/Executor/StorageFactory.php index aa9392e..edfa567 100644 --- a/src/Executor/StorageFactory.php +++ b/src/Executor/StorageFactory.php @@ -29,7 +29,7 @@ class StorageFactory * * @throws InvalidArgumentException When the connection string cannot be parsed or names a scheme with no device */ - public static function getDevice(string $root, ?string $connection = '', (ClientInterface&StreamingClientInterface)|null $client = null): Device + public static function getDevice(string $root, #[\SensitiveParameter] ?string $connection = '', (ClientInterface&StreamingClientInterface)|null $client = null): Device { $connection ??= ''; $localSchemes = ['file', DeviceType::Local->value]; @@ -40,8 +40,8 @@ public static function getDevice(string $root, ?string $connection = '', (Client try { $dsn = new DSN($connection); - } catch (\Throwable $throwable) { - throw new InvalidArgumentException('Unable to parse storage DSN: ' . $throwable->getMessage(), previous: $throwable); + } catch (\Throwable) { + throw new InvalidArgumentException('Unable to parse storage DSN'); } $scheme = $dsn->getScheme(); diff --git a/tests/unit/Executor/StorageFactoryTest.php b/tests/unit/Executor/StorageFactoryTest.php index f644e55..21659cd 100644 --- a/tests/unit/Executor/StorageFactoryTest.php +++ b/tests/unit/Executor/StorageFactoryTest.php @@ -200,6 +200,28 @@ public function testUnusableConnectionIsRefused(string $connection): void StorageFactory::getDevice('/storage/builds/app-test', $connection); } + public function testRefusalDoesNotPrintTheConnection(): void + { + $ignoreArgs = \ini_set('zend.exception_ignore_args', '0'); + $maxLength = \ini_set('zend.exception_string_param_max_len', '1000000'); + + try { + foreach (self::unusableConnections() as $name => [$connection]) { + try { + StorageFactory::getDevice('/storage/builds/app-test', $connection); + $this->fail(sprintf("Connection '%s' was accepted", $name)); + } catch (InvalidArgumentException $exception) { + $printed = (string) $exception; + $this->assertStringContainsString('/storage/builds/app-test', $printed, $name); + $this->assertStringNotContainsString($connection, $printed, $name); + } + } + } finally { + \ini_set('zend.exception_ignore_args', $ignoreArgs); + \ini_set('zend.exception_string_param_max_len', $maxLength); + } + } + /** * @return \Iterator */