From 0ca38d3a6308ad2ab28b33c3dc089d1c9c192e22 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 24 Sep 2026 19:24:11 +1200 Subject: [PATCH 1/3] fix: keep credentials out of the DSN parse failure message parse_url() rejects the mistakes people actually make in a DSN (an empty host after the userinfo, a port out of range, a secret key with an unencoded "/"), and the refusal quoted the whole DSN back, user and password included. Anything that logs the message, or lets the exception escape to stderr, wrote the credentials out with it. Nothing of the input is kept: redacting only the userinfo would mean picking apart a string parse_url() has just refused, and the query can carry secrets of its own. The scheme and host refusals already quoted nothing; the regression test pins all three. Co-Authored-By: Claude Opus 5.5 --- src/DSN/DSN.php | 2 +- tests/DSN/DSNTest.php | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/DSN/DSN.php b/src/DSN/DSN.php index fd1111b..1898b04 100644 --- a/src/DSN/DSN.php +++ b/src/DSN/DSN.php @@ -56,7 +56,7 @@ public function __construct(string $dsn) $parts = \parse_url($dsn); if (!$parts) { - throw new \InvalidArgumentException("Unable to parse DSN: $dsn"); + throw new \InvalidArgumentException('Unable to parse DSN: malformed'); } if (empty($parts['scheme'])) { diff --git a/tests/DSN/DSNTest.php b/tests/DSN/DSNTest.php index 4521f37..74060d4 100644 --- a/tests/DSN/DSNTest.php +++ b/tests/DSN/DSNTest.php @@ -7,6 +7,10 @@ class DSNTest extends TestCase { + private const USER = 'AKIAKEY'; + + private const PASSWORD = 'SECRETKEY'; + public function testSuccess(): void { $dsn = new DSN('mariadb://user:password@localhost:3306/database?charset=utf8&timezone=UTC'); @@ -153,4 +157,35 @@ public function testFail(): void $this->expectException(\InvalidArgumentException::class); new DSN('mariadb://'); } + + /** + * @return array + */ + public static function provideRefusedDSNs(): array + { + $credentials = self::USER . ':' . self::PASSWORD; + + return [ + 'unparseable' => ["s3://{$credentials}@/backups?region=us-east-1"], + 'no scheme' => ["//{$credentials}@localhost/database"], + 'no host' => ["{$credentials}@localhost/database"], + ]; + } + + /** + * @dataProvider provideRefusedDSNs + */ + public function testRefusalMessageOmitsCredentials(string $dsn): void + { + try { + new DSN($dsn); + } catch (\InvalidArgumentException $exception) { + $this->assertStringNotContainsString(self::USER, $exception->getMessage()); + $this->assertStringNotContainsString(self::PASSWORD, $exception->getMessage()); + + return; + } + + $this->fail('The DSN was accepted'); + } } From 46a8900800693c3e258707b5aaf691cf95f677f5 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 24 Sep 2026 19:24:26 +1200 Subject: [PATCH 2/3] fix: redact the DSN argument from stack traces With the message clean, the refusal's stack trace still printed the DSN argument, cut to zend.exception_string_param_max_len (15 bytes by default): the scheme, the user and the start of the password. Traces carry arguments unless zend.exception_ignore_args is on, and it is off by default and in the official PHP images, which ship no php.ini. #[\SensitiveParameter] prints the argument as Object(SensitiveParameterValue) instead. PHP before 8.2 ignores the attribute, so the trace test runs from 8.2. Co-Authored-By: Claude Opus 5.5 --- src/DSN/DSN.php | 2 +- tests/DSN/DSNTest.php | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/DSN/DSN.php b/src/DSN/DSN.php index 1898b04..58e2cbf 100644 --- a/src/DSN/DSN.php +++ b/src/DSN/DSN.php @@ -51,7 +51,7 @@ class DSN * * @param string $dsn */ - public function __construct(string $dsn) + public function __construct(#[\SensitiveParameter] string $dsn) { $parts = \parse_url($dsn); diff --git a/tests/DSN/DSNTest.php b/tests/DSN/DSNTest.php index 74060d4..ed0ee19 100644 --- a/tests/DSN/DSNTest.php +++ b/tests/DSN/DSNTest.php @@ -188,4 +188,27 @@ public function testRefusalMessageOmitsCredentials(string $dsn): void $this->fail('The DSN was accepted'); } + + /** + * @requires PHP >= 8.2 + */ + public function testUncaughtRefusalPrintsNoCredentials(): void + { + $ignoreArgs = \ini_set('zend.exception_ignore_args', '0'); + $maxLength = \ini_set('zend.exception_string_param_max_len', '1000000'); + + try { + new DSN('s3://' . self::USER . ':' . self::PASSWORD . '@/backups?region=us-east-1'); + $this->fail('The DSN was accepted'); + } catch (\InvalidArgumentException $exception) { + $printed = (string) $exception; + } finally { + \ini_set('zend.exception_ignore_args', $ignoreArgs); + \ini_set('zend.exception_string_param_max_len', $maxLength); + } + + $this->assertStringNotContainsString(self::USER, $printed); + $this->assertStringNotContainsString(self::PASSWORD, $printed); + $this->assertStringContainsString(DSN::class . '->__construct(Object(SensitiveParameterValue))', $printed); + } } From f29955fcd2e436bef54d6d0fa2c977ed40819c81 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 24 Sep 2026 19:33:38 +1200 Subject: [PATCH 3/3] fix: clear the DSN from stack traces on PHP 8.0 and 8.1 SensitiveParameter only takes effect from PHP 8.2, and this package still supports 8.0 and 8.1, where the refusal's trace kept printing the DSN argument. A trace prints a parameter's current value rather than the one passed, so the constructor drops $dsn once parse_url() is done with it, and the trace test now runs on every supported version. The test no longer pins how PHP renders a redacted argument (NULL before 8.2, Object(SensitiveParameterValue) from 8.2): it asserts that neither the user nor the password is printed. Co-Authored-By: Claude Opus 5.5 --- src/DSN/DSN.php | 2 ++ tests/DSN/DSNTest.php | 4 ---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/DSN/DSN.php b/src/DSN/DSN.php index 58e2cbf..2661c63 100644 --- a/src/DSN/DSN.php +++ b/src/DSN/DSN.php @@ -54,6 +54,8 @@ class DSN public function __construct(#[\SensitiveParameter] string $dsn) { $parts = \parse_url($dsn); + // PHP before 8.2 ignores SensitiveParameter, and stack traces print a parameter's current value + unset($dsn); if (!$parts) { throw new \InvalidArgumentException('Unable to parse DSN: malformed'); diff --git a/tests/DSN/DSNTest.php b/tests/DSN/DSNTest.php index ed0ee19..9f92fc8 100644 --- a/tests/DSN/DSNTest.php +++ b/tests/DSN/DSNTest.php @@ -189,9 +189,6 @@ public function testRefusalMessageOmitsCredentials(string $dsn): void $this->fail('The DSN was accepted'); } - /** - * @requires PHP >= 8.2 - */ public function testUncaughtRefusalPrintsNoCredentials(): void { $ignoreArgs = \ini_set('zend.exception_ignore_args', '0'); @@ -209,6 +206,5 @@ public function testUncaughtRefusalPrintsNoCredentials(): void $this->assertStringNotContainsString(self::USER, $printed); $this->assertStringNotContainsString(self::PASSWORD, $printed); - $this->assertStringContainsString(DSN::class . '->__construct(Object(SensitiveParameterValue))', $printed); } }