Proposed changing the default install/setup to prevent backing up of password in "sites/default/settings.php" file into Git Repositories.
Yes it is important to backup your"settings.php" file ... but it should not be in Git where it can accidentally be pushed into a public repo.


It seem people are checking-in/pushing in their "sites/default/settings.php" file into PUBLIC Git repositories (like github.com), if they run "git init" from the Drupal install root directory.
Obviously this is a problem because the "setting.php" file holds their database passwords.


The PATCH is to add a ".gitignore" file with the "settings*.php" pattern in the "sites/default" directory (ref http://help.github.com/git-ignore/) . This file should be included as the drupal package for all the Newbies, so by default the passwords will be protected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kvanderw’s picture

Version: 6.x-dev » 7.x-dev
Category: task » feature
Status: Active » Needs review
FileSize
1.36 KB

Additionally, I have run into situations where files were being pushed into the site and then being rolled back and rolled forward with programmer changes to the git repository. Therefore, I would propose that we also by default leave out:

files (upgraded sites from 5 or before)
sites/*/files
sites/*/private
sites/*/settings*.php

So, the goal here is to remove from git control any files that are placed into the folder structure by Drupal user and managed within the database. These kinds of files just are not eligible for 'codebase' management. And of couse, the settings*.php file originally mentioned.

rfay’s picture

Status: Needs review » Needs work

Line lengths are longer than 80 characters. There's also some trailing whitespace.

Could you reformat it? (I'd take a crack at it but getting a session ready)

+1 for this though. I think we need to do it.

Thanks!
-Randy

kvanderw’s picture

Status: Needs work » Needs review
FileSize
3.46 KB

Randy

Sorry I missed the 'rules' on the last pass... @#$#@ first timers :)

I know, I should have read all the documentation first.
I got too excited that I could actually participate personally, instead of by proxy (Kris, Zac, Steven, Jaison, Pat...)

anyway... Not sure of all the rules yet... I used this command to build the patch.
> git format-patch --stdout 074848d0 > IgnoreDBManagedFiles-917492-2.patch

That picked up both patches in one file....
Again.. not sure if that is the right way or if I should have reverted to clean and rebuilt the changes to the file so that there would be only ONE commit for the patch.

newbies!

Let me know if there is more I can do on this.

Thanks
Kurt

rfay’s picture

Status: Needs review » Needs work

Don't these lines still go over 80 chars?

Thanks to both of you for your effort on this! I think this is important.

bfroehle’s picture

Status: Needs work » Needs review

@rfay: No, I think it's just confusing since the patch is in two parts, the second of which undoes the changes in the first.

Damien Tournoud’s picture

Title: Security issue from Git in default install. » Add a sensible ,gitignore file

Renaming the issue, this is not a security issue, as much as a sensible starting point for people not to shoot themselves in the foot.

rfay’s picture

IMO this can be a little less verbose, so I trimmed it down a bit. But it looks to me like it does what we need it to do. Thanks!

chx’s picture

Why not ignore sites as it is?

rfay’s picture

sites/*/modules, sites/*/themes, sites/all may very well be added to the main repo, depending on how people organize their sites. There are many different ways to do this.

kvanderw’s picture

@rfay: I think that is the first time in 50 years of programming that I have every been told that I wrote too much documentation. Another first for me! :)

kvanderw’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me at this point.

bfroehle’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/.gitignoreundefined
@@ -0,0 +1,10 @@
\ No newline at end of file

Missing a newline at the end of the file, unless there is some reason not to include it.

Powered by Dreditor.

kvanderw’s picture

Title: Add a sensible ,gitignore file » Add a sensible .gitignore file
Status: Needs work » Needs review
FileSize
849 bytes

Ok... added the blank line at the end of the patch per docs found at http://drupal.org/coding-standards#indenting

rfay’s picture

Status: Needs review » Reviewed & tested by the community

OK, I'd say it's back to RTBC then. Thanks.

realityloop’s picture

+1 I created a patch that included settings.php, and was wondering why.. look forward to having this included

kvanderw’s picture

So, I have been thinking about this issue as I wonder about the whole process of issues and it seems to me that we aren't really finished yet.

I would think that it might be a good idea to include :
*~
*.bak
# To ignore any 'backup' files generated by various editors.

And perhaps, we should alter the whole sites thing to be more like this:
sites/
!sites/all/

In this manner, we say it with fewer lines and cover newer contingencies as they may appear...

Thoughts?

rfay’s picture

I don't agree with #16 - It's not our job to tell people what to include or not in a project. All we're trying to do is prevent the ordinary disasters we know about. Lots of people (most) will *want* to include sites/default/modules or sites/example.com/modules.

As far as artifacts like *.orig and *.bak and such... 1) People can make that decision themselves 2) the consequences aren't high.

kvanderw’s picture

@rfay

I'm good with that. I had spaced on the sites/*/modules entirely....
and probably sites/*/themes as well..

So, I'll just leave it alone at this point :)

claar’s picture

Status: Reviewed & tested by the community » Needs work

Just stumbled across this issue. -1 from me.

While the security concern is valid, this entire .gitignore file is a major What the heck/Gotcha to me and any system administrators who use git for purposes that need to include all files (migrating between dev/test/production sites, backups, migrating servers, etc.).

Particularly odd is the exclusion of sites/*/files -- and sites/*/private -- the comment says "# Ignore all the files (and folders) in these folders as they are managed by the files table." -- but I don't understand why this is relevant to whether you'd want to include them in your git repository.

The only thing I think aught to be ignored by default are the Optimized CSS and JavaScript files -- which aren't in the patch above (at least in my Drupal configuration).

If we do this, it at least should be documented somewhere obvious -- but where?

izmeez’s picture

subscribing

rfay’s picture

Status: Needs work » Needs review

I may be wrong, but believe it's standard practice to consider the files directory to be user-generated files, and not to keep them under "source" control, which is for the files that run the site. There may be a few people who use another practice, but it's not common IMO.

claar’s picture

Thanks, rfay -- that's reasonable logic to me, though I wonder how many people are thinking of git as traditional "source" control, vs. how I use it, which is site-wide version control. I do like that the above patches appear to be advocating a .gitignore at the root of the site, which is less of a gotcha than in a sub-directory.

rfay’s picture

Status: Needs review » Reviewed & tested by the community

@claar - sounds like you don't mind pushing this back to RTBC? If you do, just change it back.

I think you'll find that if you source-control your settings.php it will ruin site migrations. If you source-control your files directory, it will ruin transitions from staging to live. It can even delete appropriate files that have been added to production while your site was living on staging, depending on what you do with git.

claar’s picture

Status: Reviewed & tested by the community » Needs work

On further thought, I think the best place to document this is in the .gitignore itself, at the root of the Drupal install, so this patch handles that.

So to be clear, the future criteria for what we'll default .gitignore is something like:

  1. repositories for user-generated files
  2. drupal configuration files
  3. files that likely contain sensitive information
  4. other files that aren't recommended to keep in a git repository

If so, I suggest adding a comment to the .gitignore to explain this rational. If not a full explanation, could we improve the "managed by the files table" comment to reflect a more general criteria like the above, to set a useful precedent? If not, feel free to set this to RBTC and I'll hold my peace.

@rfay: I go from dev to staging to production (and back) with the same settings file and intact files directory, but I agree we shouldn't code for corner cases, but for best practices.

claar’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs work » Needs review
FileSize
387 bytes

Attached is an attempt to generalize the types of ignored content to make it more obvious what types of files we ignore.

No change in actual code, just comments.

I felt a full criteria definition in the file was too obnoxious, so I kept it simple.

rfay’s picture

Status: Needs review » Reviewed & tested by the community

Works for me. RTBC

kvanderw’s picture

Why is this now waiting until 8.x ??

Otherwise +1

Josh The Geek’s picture

@#27: No new features for 7.x

rfay’s picture

Umm.. I certainly wouldn't call this a new feature for D7. It's certainly not a feature. But I think our protocol is to first commit for D8, then put it back to D7. Probably it should then go to D6.

rfay’s picture

Issue tags: +Needs backport to D7
Dries’s picture

Version: 8.x-dev » 7.x-dev

I committed the patch to 8.x but removed the Drupal 5 'files' directory bit at the bottom.

I'll let @webchick decide whether she wants to include the Drupal 5 part in Drupal 7 or not.

Moving to 7.x for @webchick.

Thanks all.

yched’s picture

Problem is we now can't edit .gitignore for our own needs (mine is set to ignore *.patch and .gitignore), without those .gitignore changes being included in all the patches we roll ?

rfay’s picture

@yched, I'd recommend just making a commit to your local that changes your .gitignore.

@Dries, the reason /files is in there is that there are a lot of sites (like drupal.org) that still have all their files in /files. I imagine there still will be in Drupal 8.

yched’s picture

@rfay - doh, sure. Thanks :-)

webchick’s picture

Version: 7.x-dev » 8.x-dev
Category: feature » task
Status: Reviewed & tested by the community » Needs review

Yeah, I think this is still on the table for D7. It's way too easy to do git add . and end up getting settings.php changes in your patches, among other things.

The D5 files part, hm. I'm not sure. While a common configuration, it's not the default configuration for D6, and that's technically the only version D7 supports upgrading from.

I committed to 7.x the same .gitignore file as 8.x (e.g. without the /files bit); now bumping back to D8 and "needs review" to talk more about that aspect and whether it should belong.

webchick’s picture

Also, why sites/*/private? For private files, we recommend those be located outside the web root, where they'd be unaffected by Git.

We briefly, circa beta1 or something, created the private files directory automatically under sites/default, but stopped doing that for security. So I'm not sure that bit belongs in this file either.

rfay’s picture

For those who are worried about how to maintain a private version of .gitignore:
1. You can set

git config --global core.excludesfile ~/.gitignore

and put anything you want to globally ignore in all repos in there.

Edit:
You can also in any git repo:

git config core.excludesfile .mygitignore

and then put whatever private stuff you want in there.

2. You can have .gitignores in *any* subdirectory.

3. You can change the gitignore and check it in. It perhaps will annoy you that your tracking branch has something in it that is not in the upstream branch.

@webchick: sites/default/private is the de facto standard for where the private files goes, because we allow it and offer it as the default. I think it makes sense. And it does absolutely *no* harm to have it in the .gitignore if it doesn't exist. And would it have VCS-trackable files in it if it did exist?

kvanderw’s picture

@webchick, so, you're saying if someone upgrades from D5 to D6, and 'files' is left alone, the upgrade will fail to D7 ?? Or, did the D7 upgrade migrate those files and update the tables to point to the new location?

I think I'll go read some code for a while.

And, being engrossed in too much of my own world, who needs to backport this to D6 ??

also, should the status now be "patch (to be ported)" ??

kvanderw’s picture

Issue tags: -Needs backport to D6

Apparently, I can't edit an Issue tag if I get it wrong :)

rfay’s picture

Version: 8.x-dev » 6.x-dev
Status: Needs review » Patch (to be ported)
Issue tags: +Needs backport to D6

IMO it would be good to get this into D6. It's just an artifact of our migration to git that needs to be accounted for.

I'm not at all sure we need to give this any more attention in D8 or D7. It's good enough. No bikeshedding needed about /files. This is all people's own site management stuff. We're just trying to make it harder to shoot yourself in the foot, or the head.

Setting to D6, patch to be ported. @kvanderw, want to update it? I think just removing the sites/*/private and adding back in the /files (which is quite common on d6 sites) would do it.

webchick’s picture

Version: 6.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs review
Issue tags: -Needs backport to D6

"so, you're saying if someone upgrades from D5 to D6, and 'files' is left alone, the upgrade will fail to D7 ??"

Hm? No, I definitely didn't say that? I said that Drupal hasn't defaulted to storing files in /files since Drupal 5, and it's typically bad form to store workarounds for Drupal 5- in Drupal 7+. The D6 upgrade path doesn't do anything about relocating the files, afaik, so all Drupal sites from before D6 will have their files in /files if they haven't changed anything.

So all that will currently happen if your files are stored in /files is Git won't ignore it when you type something like git add . You'd need to do a custom .gitignore as rfay outlines.

rfay: The Private files directory doesn't default to anything. And the help text for that field says explicitly "This directory should not be accessible over the web."

Private files screenshot

So I would recommend removing that line, as well as adding some docs to the effect of #37 to help folks who need to customize this further.

webchick’s picture

Version: 8.x-dev » 6.x-dev
Status: Needs review » Patch (to be ported)
Issue tags: +Needs backport to D6

Sorry. Cross-post.

Crell’s picture

I should note that the patch in #25 breaks the de-facto standard, used by d.o sites as well, of settings.php for site-local settings and settings.local.php for server-local settings (database, API keys, etc.) A lot of sites do that for perfectly good reason, including d.o, and that ignore file breaks it.

(Side note: I'm very glad right now that I use drush, not git, to manage my projects and do *not* tie into the Drupal git repos directly...)

Dave Reid’s picture

Subscribe, this file makes it much more difficult for people that have Drupal checked out for core development, and modules that they maintain checked out into sites/all/modules as well. Also, I'm the 20% minority, but not being able to alter the base .gitignore file without now 'hacking core' is frustrating. :(

bfroehle’s picture

Should we put .gitignore in .gitignore?

Crell’s picture

We shouldn't have a .gitignore file in core itself, IMO.

rfay’s picture

Before we get a full-scale rebellion going, I blogged on techniques to work with a core .gitignore: http://randyfay.com/node/102

Crell’s picture

Thanks, rfay. But that doesn't do anything about the fact that the current ignore file actively blocks the settings.local.php technique used by Drupal.org itself, that many others have started adopting exactly because Drupal.org uses it. That's doubleplusungood.

Dave Reid’s picture

rfay’s picture

I assume you're saying that settings.local.php should be uncontrolled, but settings.php should be controlled. (settings.local.php as used by d.o appears to be to be *exactly* what should be ignored in source control)

I do suspect we should replace default.settings.php by settings.php and use the settings.local.php technique. But that's quite a ways beyond the scope of this issue.

rfay’s picture

Oh, and @davereid, #1118520: Add inclusion of a settings.local.php file in settings.php can just make appropriate changes to the gitignore for its new strategy, can't it?

webchick’s picture

Please see #917492: Add a sensible .gitignore file. This change was pretty controversial in 7.2, and I'd like to see some of the folks from this issue chime in over there before we commit this to 6.x too.

webchick’s picture

jensimmons’s picture

I'd suggest adding
.DS_Store
to the git-ignore file. Just as a courtesy to Mac users. It's a total PITA to end up with a repo full of .DS_Store files before remembering to make the gitignore.

Pantheon includes this gitignore in their standard install:

# Drupal #
##########
sites/*/files
sites/*/files/*
files/*

# Packages #
############
*.7z
*.dmg
*.gz
*.iso
*.jar
*.rar
*.tar
*.zip

# Logs and databases #
######################
*.log
*.sql

# OS generated files #
######################
.DS_Store?
ehthumbs.db
Icon?
Thumbs.db
._*

I always remove the '?' from '.DS_Store?'. (Not sure why there's a question about it.)

Food for thought.

AlexBorsody’s picture

github generates this, it works well

sites/default/files
sites/default/private
sites/default/settings.php
cache/
files/
/README.txt
/CHANGELOG.txt
/COPYRIGHT.txt
/INSTALL*.txt
/LICENSE.txt
/MAINTAINERS.txt
/UPGRADE.txt
robots.txt
sites/all/README.txt
sites/all/modules/README.txt
sites/all/themes/README.txt
.htaccess

#for non core developer
#only include "sites" folder without exclusions before
cron.php
index.php
install.php
update.php
xmlrpc.php
/includes
/misc
/modules     
/profiles
/scripts
/themes
RoloDMonkey’s picture

I always remove the '?' from '.DS_Store?'. (Not sure why there's a question about it.)

In case you weren't joking, the question mark means "one optional character", the same way that the asterix means "multiple optional characters".

For Icon?, it will match on 'Icon' or 'Icons'. Presumably, there is a similar variant, '.DS_Stores' perhaps?

the_g_bomb’s picture

I just installed D8 to help out with some testing. Thought it would just be install and start testing patches etc. I must admit I was a bit surprise when my git immediately told me that there was untracked files, which is when I noticed there wasn't a .gitignore file but there was an example.gitignore, that makes sense I thought, just like the default.setting.php file.

I was even more surprised when, on creating the .gitignore file from the example, I still got this:

# On branch 8.x
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#	.gitignore

I would like to add a .gitignore in the .gitignore example file, even if its commented out as a reminder.

ShaunDychko’s picture

If you add:

# Ignore configuration files that may contain sensitive information.
sites/*/settings*.php

# Ignore paths that contain user-generated content.
sites/*/files
sites/*/private

to ~/.gitignore_global
then the settings.php files and user generated content will always be ignored, without needing to rename example.gitignore to .gitignore each time Drupal is updated.

mattew’s picture

Issue summary: View changes

Hi,

Sorry to ask this question that seems totally borne by all, but I would like somebody explain to me why settings.php could not be under version control.
The only valuable argument for me is that we should not put sensible informations under version control, I think you talk about database access. I'm OK for that, but let me develop.

It's very usefull in my mind to keep settings.php under version control, I can describe many case where it's useful, for example :
1. When you want to set environment-specific parameters (like variables for example)
2. When you host your files in production on an environment where you can't act on the files but just deploy using Git (switching, mergin, tagging, etc.). I think about :
- Hosting like Acquia Cloud : we can't touch the files, we have to deploy using tags and branches.
- Hosting managed by the customer : it's a very common case for big customer who want to host there website on there own server. I can assure you that it's a pain when we ask them to change a line of code in a PHP file, as we almost never talk to someone who knows PHP.
Most of the time in our web agency, hosting administrators are the only person who can touch unversionned files on production environment. I can not think about the idea to ask them every time I want to add a line in settings.php. If you could know my hosting administrator, you would understand why... But I just want to keep control on the file, and I think it's understandable.

So, in this kind of cases, which for me are not uncommon, we have to put settings.php under version control.

For the considerations about database credentials, I think this particular informations could be keep out of the settings.php, using a PHP-included file, and this file could be managed by the administrator of the hosting if needed. It's the way it works on Acquia Cloud.
And for me it's obviously not difficult to keep folders clean under sites directory, using the sites.php file and proper folders names. We could think about an example (which is near how we are working in our web agency) :
- sites/default/settings.php not versionned, allowing developpers to set there own, local settings.
- sites/production/settings.php for production settings
- sites/staging/settings.php for staging ...
and so on.

Is it an heresy to work like this ?

For me, saying that we should not put settings.php under version control sounds very dogmatic... it's not difficult to do it wisely, and it avoid lots of pain when it's time to act on this files and to test and deploy modifications, so why telling everybody that it's bad ? Drupal multi-sites structure is well adapted for this kind of deployments accross multiple environments, and it's easy to use. Why should we not rely on it ?

Should I understand that it's just a recommendation for beginners to avoid breaking their site ? Or everybody really think that my arguments are wrong and I definitely should not put settings.php under version control ?

Thanks for your wise advices.

realityloop’s picture

@matthew

It's generally considered bad practice to keep settings.php under version control because it contains username and password for your database which is likley different for different environments (https://drupal.org/node/803746#gitignore), we don't capture it in our own workflow as we use different mysql user/pass between out dev/staging/prod servers.

That said it may work for you to edit the .gitignore once you have downloaded drupal core to allow settings.php to be tracked, it just depends on your workflow overall.

You might like to look at adding an include to your settings.php like Aegir uses to for local.settings.php. Then all of the non-sensitive additions, domain access for instance, can be stored in that instead, and that file can be tracked.

Crell’s picture

Everyone I know uses the settings.local.php include file trick, included from settings.php, for non-versioned data like DB settings. That's even supported out of the box in Drupal 8.

mattew’s picture

Thanks for your answers. I'm glad to see that a smart version control of settings.php (i.e. not including db credentials) is not seen as an heresy!

@Crell
Do you mean that it will be the recommendation in Drupal 8 to separate environment-dependent settings (settings.php under version control) and db settings (settings.local.php NOT under V.C.) ? You are talking about this kind of practice: http://www.metaltoad.com/blog/stage-specific-drupal-settings-localsettin... ?

If I understand well this article http://community.aegirproject.org/node/71 , Aegir works like that :
- sites/mydomain/local.settings.php are under VC, and contain site-specific/environment-specific parameters
- /var/aegir/config/includes/global.inc is not under VC (is out of the website file system) and contains global parameters ?

But:
1. Where are the Drupal settings.php in this configuration ? Are they under VC ?
2. Does /var/aegir/config/includes/global.inc contains db credentials ?

This kind of configurations for big websites or big platforms, could be documented, as a kind of "advanced best practice" somewhere, maybe ?

I'm sorry, this is not the main purpose of this thread... Maybe should I ask somewhere else ?

Thanks a lot

Status: Patch (to be ported) » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.