cakephp / phinx

PHP Database Migrations for Everyone
https://phinx.org
MIT License
4.45k stars 895 forks source link

Mark password as a SensitiveParameter #2295

Closed edudobay closed 1 month ago

edudobay commented 1 month ago

Mark the $password parameter as sensitive, so its value is redacted if present in a stack trace. This already happens for PDO but not for this method:

#0 /src/vendor/robmorgan/phinx/src/Phinx/Db/Adapter/PdoAdapter.php(96):
  PDO->__construct('pgsql:dbname=aa...', 'my-username', Object(SensitiveParameterValue), Array)
#1 /src/vendor/robmorgan/phinx/src/Phinx/Db/Adapter/PostgresAdapter.php(115):
  Phinx\Db\Adapter\PdoAdapter->createPdoConnection('pgsql:dbname=aa...', 'my-username', 'my password leaked...', Array)
MasterOdin commented 1 month ago

SensitiveParameter was added in PHP 8.2, is there any issue with using this in PHP 8.1 which we still support? I'm not super well versed in attributes, and from test suite I guess it works and the attribute is ignored if it doesn't exist?

e: playing around with a simple snippet, does seem like older versions of PHP will just ignore the attribute and continue to work the same, so this won't break anything, but is also only applicable to people running PHP 8.2+.

edudobay commented 1 month ago

Yes, that's exactly what I expected, it only benefits people running PHP 8.2+, but should not impact older versions in any way.