Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
raffaelj opened this issue Mar 14, 2024 · 1 comment

Comments

@raffaelj
Copy link

raffaelj commented Mar 14, 2024

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

  • install Drupal with SQLite
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/',
);
  • run drush archive:dump.

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant