drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.33k stars 1.08k forks source link

archive:dump complains about database credentials in settings.php when using SQLite #5910

Open raffaelj opened 5 months ago

raffaelj commented 5 months ago

Describe the bug

I can't archive my web/sites/default/settings.php because it contains database settings (without any password) for SQLite.

To Reproduce

mkdir test && cd test

composer create-project --no-install drupal/recommended-project .
composer require --no-install drush/drush
# add/remove other packages, add vcs repos

# add folders
mkdir -p {db,config/sync,patches,tmp,private}

# initial composer installation
composer install --no-dev --optimize-autoloader

# add drush to $PATH
# echo PATH=\$PATH:$PWD/vendor/bin >> ~/.bash_profile

# initial installation
./vendor/bin/drush site:install --yes --db-url="sqlite://../db/db.sqlite" --locale="de" --site-name="Test" --config-dir="../config/sync" --site-mail="test@example.com" --account-mail="test@example.com" --account-name="test"

# set missing config to match folder structure
chmod u+w web/sites/default/settings.php
echo "\$settings['config_sync_directory'] = '../config/sync';" >> web/sites/default/settings.php
echo "\$settings['file_temp_path'] = '../tmp';" >> web/sites/default/settings.php
echo "\$settings['file_private_path'] = '../private';" >> web/sites/default/settings.php

# other settings, disable modules, etc.

generated db settings in settings.php:

$databases['default']['default'] = array (
  'database' => '../db/db.sqlite',
  'prefix' => '',
  'driver' => 'sqlite',
  'namespace' => 'Drupal\\sqlite\\Driver\\Database\\sqlite',
  'autoload' => 'core/modules/sqlite/src/Driver/Database/sqlite/',
);

Error message:

In ArchiveDumpCommands.php line 557:
Found database connection settings in web/sites/default/settings.php. It is risky to include them to the archiv  
  e. Please move the database connection settings into a setting.*.php file or exclude them from the archive with  
   "--exclude-code-paths=web/sites/default/settings.php".

Expected behavior

Because I use SQLite, settings.php should be archived without a warning.

Actual behavior

I can't archive settings.php.

Workaround

I could create a settings.db.php file and include that from settings.php. When doing so, the archive is created. But than I would have to rsync settings.db.php to keep the correct database location, so I could use rsync in the first place.

Because I immediatly ran into the next error (see below) after resolving this issue with a settings.db.php in an existing project with content and images, I'll probably switch to native tools like mysqldump and rsync instead of using drush for imports, exports and backups.

So the workaround is to not use drush.

Error mentioned above because of default memory limit in php docker container:

Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 138998272 bytes) in /var/www/vendor/drush/drush/src/Commands/core/ArchiveDumpCommands.php on line 173

System Configuration

Q A
Drush version? 12.4.4 (latest)
Drupal version? 10.2.4 (latest)
PHP version 8.1 and 8.2
OS? Linux

Additional information

There is a typo in "Please move the database connection settings into a setting.*.php", which should be "...settings.*.php".

raffaelj commented 5 months ago

Another use case, when the archive:dump command stops with a false positive:

$databases['default']['default'] = array(
  'driver' => 'mysql',
  'database' => 'drupaldb1',
  'username' => getenv('DB_USER'),
  'password' => getenv('DB_PASSWORD'),
  'host' => 'dbserver1',
);

Another workaround: Read the regexes of the source file and try to trick it.

$db = array (
  'database' => '../db/db.sqlite',
  'prefix' => '',
  'driver' => 'sqlite',
  'namespace' => 'Drupal\\sqlite\\Driver\\Database\\sqlite',
  'autoload' => 'core/modules/sqlite/src/Driver/Database/sqlite/',
);
$databases['default']['default'] = $db;

Possible solutions:

  1. change regex in ArchiveDumpCommands.php to match all possible edge cases
  2. add cli argument to skip that security check, e. g. --skip-db-settings-check
  3. remove the validateSensitiveData() method entirely

Variant 1 seems over complicated and it would be impossible to match every possible edge case. 2 would be a good option to keep everything as it is, but with a workaround to use the command at all when caring about password security in different ways or when using SQLite.

I would vote for 3 because the current check gives a false sense of security when using any non-default code patterns like the workaround above.