Improve error message and give remedy intructions - "It's a security risk to have a backup folder in your Drupal root"

We need better error message there.

Just move it out of site root and then you will see that by default drush uses subdirs under ~/.drush-backups. It tells you exactly where your code/data went *after* the backup completes. you can put all your old backups under there if you wish.

For custom location, use --backup-dir which is documented in the help for pm-updatecode.

Does this security issue apply to other files as well? For instance, I use the backup folder for backup and migrate files as well. If so, a much more comprehensive error message needs to be displayed.

Does this security issue apply to other files as well? For instance, I use the backup folder for backup and migrate files as well. If so, a much more comprehensive error message needs to be displayed.

In addition, drush will not complete even if a blank backup directory is present, even if there are not modules there. This forces the user to move backup and migrate folders or rename the directory.

Any drush command from a contrib module like Backup and migrate is responsible for its own error messages. And we are not going to make accommodations for its poor practices. Here, we are talking about pm-updatecode and sql-dump commands.

Any drush command from a contrib module like Backup and migrate is responsible for its own error messages.

What I'm saying is that the drush error message, "It's a security risk to have a backup folder in your Drupal root" occurs even if there are no modules backed up in a "backup" directory. So drush prompts an error for a directory that has been created for the benefit of another module, NOT for drush itself.

That makes this a problem with Drush, not backup_migrate. In order to get drush to operate correctly, I had to completely remove any directory in my web root that had the name "backup" even if there was not a "modules" subdirectory within it for drush.

Are you saying that this error message is not generated by drush?

Just move it out of site root and then you will see that by default drush uses subdirs under ~/.drush-backups

I have deleted the backup folder in the web root, but after running "drush up," I have no backup modules in ~/.drush-backups. There is no .drush-backups folder anywhere on my system, so all my old modules are now gone. If this directory must be created manually, then the README.txt file should mention that.

@pukku this was a commit related to a security advisory we received. So there's no issue. You're right: there's no explicit site-based separation of backed up projects, but they are stored in date based directories and will never mix one site with another.

@zoon_unit the message is printed if backups directory exists. Also I suppose you're aware of the VERY IMPORTANT SECURITY NOTE in backup_migrate's readme.txt

Attached patch improve the error message and also allow users to still use $drupal_root/backup:

$ /usr/src/drush-HEAD-clean/drush up  
Backup directory /var/www/drupal-6.x-cvs/backup found. It's a security risk to store backups inside the Drupal tree. Drush now uses by default    [error]
~/.drush-backups. You need to move /var/www/drupal-6.x-cvs/backup out of the Drupal tree to proceed. Note: if you know what you're doing you can
explicitly set --backup-dir to /var/www/drupal-6.x-cvs/backup and continue. Tip: add $options['backup-dir'] = './backup'; in drupalrc.php in your
Drupal root.
An error occurred at function : drush_pm_updatecode_validate                                                                                      [error]
An error occurred at function : drush_pm_update                                                                                                   [error]

Committed without the Tip. I'm worried someone will get their backup stolen. Thanks for the better msg here.

Thanks for the quick response. The new approach makes sense, but what is the situation with ~/.drush-backups? Is this a directory we must create manually and set a variable somewhere? Or is it supposed to be created automatically?

it is created by drush. See http://api.drush.ws/api/drush/commands--pm--version_control--backup.inc/...

it is created by drush. See http://api.drush.ws/api/drush/commands--pm--version_control--backup.inc/...

Then there is a bug, because the directory ".drush-backups" was not created anywhere on my system.

Should I submit this as a new issue?

@zoon_unit .drush-backups is to be created automatically in the home directory of the user running drush (that is, ~/.drush-backups). If drush is unable to create it, or it does exist but is not writable an error will be set. If this is not working for you, create a new issue with the complete output with --debug.

I'm not getting any errors. In fact, Drush blissfully updates modules just fine. It just doesn't write the old modules to .drush-backups. (since the directory doesn't exist) I've checked my home directory as well as everywhere else on the server: no .drush-backups.

I do have a somewhat nonstandard Drush install. My drush command resides in .drush/drush/ under my home directory. So my directory tree is:
/home/myusername/.drush/drush/drush

Drush make commands are in:
/home/myusername/.drush/drush_make/

I'm starting to wonder if this is the culprit...

Almost all sites have *some* custom code. Furthermore, sql-dump --resultfile now dumps to the backup folder by default. So your data could be exposed.

I do agree that the folder hierarchy inside of .drush-backups could be improved. please suggest something in a new issue. also consider that db dumps and drush selfupdate backups use the backup folder as well.