Flag parameters as sensitive

… if they could contain the database password.
doctrine · Jan 24, 2023 · e1aa994 · e1aa994
1 parent 824b327
commit e1aa994
Show file tree

Hide file tree

Showing 27 changed files with 222 additions and 154 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -101,6 +101,7 @@ parameters:
             message: '~^Casting to bool something that''s already bool\.$~'
             paths:
                 - src/Connections/PrimaryReadReplicaConnection.php
+                - src/Driver/SQLite3/Driver.php
 
         # Type check for legacy implementations of the Connection interface
         # TODO: remove in 4.0.0

diff --git a/src/Cache/QueryCacheProfile.php b/src/Cache/QueryCacheProfile.php
@@ -117,9 +117,7 @@ public function getCacheKey()
      */
     public function generateCacheKeys($sql, $params, $types, array $connectionParams = [])
     {
-        if (isset($connectionParams['password'])) {
-            unset($connectionParams['password']);
-        }
+        unset($connectionParams['password']);
 
         $realCacheKey = 'query=' . $sql .
             '&params=' . serialize($params) .

diff --git a/src/Connection.php b/src/Connection.php
@@ -25,6 +25,7 @@
 use Doctrine\DBAL\Types\Type;
 use Doctrine\Deprecations\Deprecation;
 use LogicException;
+use SensitiveParameter;
 use Throwable;
 use Traversable;
 
@@ -173,6 +174,7 @@ class Connection
      * @throws Exception
      */
     public function __construct(
+        #[SensitiveParameter]
         array $params,
         Driver $driver,
         ?Configuration $config = null,
@@ -1099,7 +1101,7 @@ public function executeCacheQuery($sql, $params, $types, QueryCacheProfile $qcp)
         }
 
         $connectionParams = $this->params;
-        unset($connectionParams['platform']);
+        unset($connectionParams['platform'], $connectionParams['password'], $connectionParams['url']);
 
         [$cacheKey, $realKey] = $qcp->generateCacheKeys($sql, $params, $types, $connectionParams);
 

diff --git a/src/Connections/PrimaryReadReplicaConnection.php b/src/Connections/PrimaryReadReplicaConnection.php
@@ -15,6 +15,7 @@
 use Doctrine\DBAL\Statement;
 use Doctrine\Deprecations\Deprecation;
 use InvalidArgumentException;
+use SensitiveParameter;
 
 use function array_rand;
 use function count;
@@ -263,8 +264,11 @@ protected function connectTo($connectionName)
      *
      * @return mixed
      */
-    protected function chooseConnectionConfiguration($connectionName, $params)
-    {
+    protected function chooseConnectionConfiguration(
+        $connectionName,
+        #[SensitiveParameter]
+        $params
+    ) {
         if ($connectionName === 'primary') {
             return $params['primary'];
         }

diff --git a/src/Driver.php b/src/Driver.php
@@ -7,23 +7,30 @@
 use Doctrine\DBAL\Driver\Exception;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Schema\AbstractSchemaManager;
+use SensitiveParameter;
 
 /**
  * Driver interface.
  * Interface that all DBAL drivers must implement.
+ *
+ * @psalm-import-type Params from DriverManager
  */
 interface Driver
 {
     /**
      * Attempts to create a connection with the database.
      *
-     * @param mixed[] $params All connection parameters.
+     * @param array<string, mixed> $params All connection parameters.
+     * @psalm-param Params $params All connection parameters.
      *
      * @return DriverConnection The database connection.
      *
      * @throws Exception
      */
-    public function connect(array $params);
+    public function connect(
+        #[SensitiveParameter]
+        array $params
+    );
 
     /**
      * Gets the DatabasePlatform instance that provides all the metadata about

diff --git a/src/Driver/AbstractOracleDriver.php b/src/Driver/AbstractOracleDriver.php
@@ -54,7 +54,7 @@ public function getExceptionConverter(): ExceptionConverter
     /**
      * Returns an appropriate Easy Connect String for the given parameters.
      *
-     * @param mixed[] $params The connection parameters to return the Easy Connect String for.
+     * @param array<string, mixed> $params The connection parameters to return the Easy Connect String for.
      *
      * @return string
      */

diff --git a/src/Driver/AbstractSQLiteDriver/Middleware/EnableForeignKeys.php b/src/Driver/AbstractSQLiteDriver/Middleware/EnableForeignKeys.php
@@ -6,6 +6,7 @@
 use Doctrine\DBAL\Driver\Connection;
 use Doctrine\DBAL\Driver\Middleware;
 use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
+use SensitiveParameter;
 
 class EnableForeignKeys implements Middleware
 {
@@ -15,8 +16,10 @@ public function wrap(Driver $driver): Driver
             /**
              * {@inheritDoc}
              */
-            public function connect(array $params): Connection
-            {
+            public function connect(
+                #[SensitiveParameter]
+                array $params
+            ): Connection {
                 $connection = parent::connect($params);
 
                 $connection->exec('PRAGMA foreign_keys=ON');

diff --git a/src/Driver/IBMDB2/DataSourceName.php b/src/Driver/IBMDB2/DataSourceName.php
@@ -4,6 +4,8 @@
 
 namespace Doctrine\DBAL\Driver\IBMDB2;
 
+use SensitiveParameter;
+
 use function implode;
 use function sprintf;
 use function strpos;
@@ -15,8 +17,10 @@ final class DataSourceName
 {
     private string $string;
 
-    private function __construct(string $string)
-    {
+    private function __construct(
+        #[SensitiveParameter]
+        string $string
+    ) {
         $this->string = $string;
     }
 
@@ -30,8 +34,10 @@ public function toString(): string
      *
      * @param array<string,mixed> $params
      */
-    public static function fromArray(array $params): self
-    {
+    public static function fromArray(
+        #[SensitiveParameter]
+        array $params
+    ): self {
         $chunks = [];
 
         foreach ($params as $key => $value) {
@@ -46,8 +52,10 @@ public static function fromArray(array $params): self
      *
      * @param array<string,mixed> $params
      */
-    public static function fromConnectionParameters(array $params): self
-    {
+    public static function fromConnectionParameters(
+        #[SensitiveParameter]
+        array $params
+    ): self {
         if (isset($params['dbname']) && strpos($params['dbname'], '=') !== false) {
             return new self($params['dbname']);
         }

diff --git a/src/Driver/IBMDB2/Driver.php b/src/Driver/IBMDB2/Driver.php
@@ -4,6 +4,7 @@
 
 use Doctrine\DBAL\Driver\AbstractDB2Driver;
 use Doctrine\DBAL\Driver\IBMDB2\Exception\ConnectionFailed;
+use SensitiveParameter;
 
 use function db2_connect;
 use function db2_pconnect;
@@ -15,8 +16,10 @@ final class Driver extends AbstractDB2Driver
      *
      * @return Connection
      */
-    public function connect(array $params)
-    {
+    public function connect(
+        #[SensitiveParameter]
+        array $params
+    ) {
         $dataSourceName = DataSourceName::fromConnectionParameters($params)->toString();
 
         $username      = $params['user'] ?? '';

diff --git a/src/Driver/Middleware/AbstractDriverMiddleware.php b/src/Driver/Middleware/AbstractDriverMiddleware.php
@@ -8,6 +8,7 @@
 use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\VersionAwarePlatformDriver;
 use Doctrine\Deprecations\Deprecation;
+use SensitiveParameter;
 
 abstract class AbstractDriverMiddleware implements VersionAwarePlatformDriver
 {
@@ -21,8 +22,10 @@ public function __construct(Driver $wrappedDriver)
     /**
      * {@inheritdoc}
      */
-    public function connect(array $params)
-    {
+    public function connect(
+        #[SensitiveParameter]
+        array $params
+    ) {
         return $this->wrappedDriver->connect($params);
     }
 

diff --git a/src/Driver/Mysqli/Driver.php b/src/Driver/Mysqli/Driver.php
@@ -8,10 +8,10 @@
 use Doctrine\DBAL\Driver\Mysqli\Initializer\Charset;
 use Doctrine\DBAL\Driver\Mysqli\Initializer\Options;
 use Doctrine\DBAL\Driver\Mysqli\Initializer\Secure;
+use Generator;
 use mysqli;
 use mysqli_sql_exception;
-
-use function count;
+use SensitiveParameter;
 
 final class Driver extends AbstractMySQLDriver
 {
@@ -20,8 +20,10 @@ final class Driver extends AbstractMySQLDriver
      *
      * @return Connection
      */
-    public function connect(array $params)
-    {
+    public function connect(
+        #[SensitiveParameter]
+        array $params
+    ) {
         if (! empty($params['persistent'])) {
             if (! isset($params['host'])) {
                 throw HostRequired::forPersistentConnection();
@@ -32,27 +34,9 @@ public function connect(array $params)
             $host = $params['host'] ?? null;
         }
 
-        $flags = 0;
-
-        $preInitializers = $postInitializers = [];
-
-        if (isset($params['driverOptions'])) {
-            $driverOptions = $params['driverOptions'];
-
-            if (isset($driverOptions[Connection::OPTION_FLAGS])) {
-                $flags = $driverOptions[Connection::OPTION_FLAGS];
-                unset($driverOptions[Connection::OPTION_FLAGS]);
-            }
-
-            $preInitializers = $this->withOptions($preInitializers, $driverOptions);
-        }
-
-        $preInitializers  = $this->withSecure($preInitializers, $params);
-        $postInitializers = $this->withCharset($postInitializers, $params);
-
         $connection = new mysqli();
 
-        foreach ($preInitializers as $initializer) {
+        foreach ($this->compilePreInitializers($params) as $initializer) {
             $initializer->initialize($connection);
         }
 
@@ -64,7 +48,7 @@ public function connect(array $params)
                 $params['dbname'] ?? null,
                 $params['port'] ?? null,
                 $params['unix_socket'] ?? null,
-                $flags,
+                $params['driverOptions'][Connection::OPTION_FLAGS] ?? 0,
             );
         } catch (mysqli_sql_exception $e) {
             throw ConnectionFailed::upcast($e);
@@ -74,67 +58,60 @@ public function connect(array $params)
             throw ConnectionFailed::new($connection);
         }
 
-        foreach ($postInitializers as $initializer) {
+        foreach ($this->compilePostInitializers($params) as $initializer) {
             $initializer->initialize($connection);
         }
 
         return new Connection($connection);
     }
 
     /**
-     * @param list<Initializer> $initializers
-     * @param array<int,mixed>  $options
+     * @param array<string, mixed> $params
      *
-     * @return list<Initializer>
+     * @return Generator<int, Initializer>
      */
-    private function withOptions(array $initializers, array $options): array
-    {
-        if (count($options) !== 0) {
-            $initializers[] = new Options($options);
+    private function compilePreInitializers(
+        #[SensitiveParameter]
+        array $params
+    ): Generator {
+        unset($params['driverOptions'][Connection::OPTION_FLAGS]);
+
+        if (isset($params['driverOptions']) && $params['driverOptions'] !== []) {
+            yield new Options($params['driverOptions']);
         }
 
-        return $initializers;
-    }
-
-    /**
-     * @param list<Initializer>   $initializers
-     * @param array<string,mixed> $params
-     *
-     * @return list<Initializer>
-     */
-    private function withSecure(array $initializers, array $params): array
-    {
         if (
-            isset($params['ssl_key']) ||
-            isset($params['ssl_cert']) ||
-            isset($params['ssl_ca']) ||
-            isset($params['ssl_capath']) ||
-            isset($params['ssl_cipher'])
+            ! isset($params['ssl_key']) &&
+            ! isset($params['ssl_cert']) &&
+            ! isset($params['ssl_ca']) &&
+            ! isset($params['ssl_capath']) &&
+            ! isset($params['ssl_cipher'])
         ) {
-            $initializers[] = new Secure(
-                $params['ssl_key']    ?? '',
-                $params['ssl_cert']   ?? '',
-                $params['ssl_ca']     ?? '',
-                $params['ssl_capath'] ?? '',
-                $params['ssl_cipher'] ?? '',
-            );
+            return;
         }
 
-        return $initializers;
+        yield new Secure(
+            $params['ssl_key']    ?? '',
+            $params['ssl_cert']   ?? '',
+            $params['ssl_ca']     ?? '',
+            $params['ssl_capath'] ?? '',
+            $params['ssl_cipher'] ?? '',
+        );
     }
 
     /**
-     * @param list<Initializer>   $initializers
-     * @param array<string,mixed> $params
+     * @param array<string, mixed> $params
      *
-     * @return list<Initializer>
+     * @return Generator<int, Initializer>
      */
-    private function withCharset(array $initializers, array $params): array
-    {
-        if (isset($params['charset'])) {
-            $initializers[] = new Charset($params['charset']);
+    private function compilePostInitializers(
+        #[SensitiveParameter]
+        array $params
+    ): Generator {
+        if (! isset($params['charset'])) {
+            return;
         }
 
-        return $initializers;
+        yield new Charset($params['charset']);
     }
 }