I'd like to suggest that ...

function provisionacl_site_acls() {
  $group = d()->client_name;
  $dirs = array('modules', 'themes', 'libraries', 'files', 'private');

be changed to add ".git" as a folder that can have group perms.

function provisionacl_site_acls() {
  $group = d()->client_name;
  $dirs = array('modules', 'themes', 'libraries', 'files', 'private', '.git');

This way, developers can work on a cloned site and commit changes. Some folks will want separate repos for modules/themes etc, but some might want the repo to be a level up with a single .gitignore file to prevent files and settings from getting committed. Just less repos to manage when you have hundreds of sites to keep track of.

Comments

mvc’s picture

Title: add support for .git » give access to entire site directory (was: add support for .git)

right, the larger problem is that we don't grant access to the entire sitedir. today i needed to create a local.settings.php for a site, tomorrow i may want to add a README.txt about a client site for my collegues, and the day after i may want to add .git & .gitignore to that directory. anarcat says it's reasonable to just grant access to the sitedir, so we'll just do that. problem solved!

anarcat’s picture

Status: Active » Needs review
chrisschaub’s picture

Nice! Thanks, I'll try it out.

chrisschaub’s picture

Can I use the latest dev checkout of provisionacl with aegir 1.6?

anarcat’s picture

Status: Needs review » Fixed

Yes, you should.

I'll close this issue now as I'll just ship this with provisionacl 1.7

mvc’s picture

Status: Fixed » Active

Hmm, this still needs work. Because the change to the site directory isn't recursive, files within it which are not explicitly handled will not be accessible after the site is migrated, for example. This change makes it possible to create .git or local.settings.php, but after a migration the user won't be able to edit them. We can either continue explicitly listing files and directories which users should be able to write to, or just grant access to the entire sitedir recursively. I think the second makes sense, on the grounds that migrating shouldn't remove my ability to edit a file I made.

I can't think of a use case in which the user shouldn't have write access to the contents of this directory. They can already delete settings.php or drushrc.php and re-create it with any changes they want.

chrisschaub’s picture

Edited: This is incorrect!

The verify seems to set the permissions correctly for the client? I've migrated and changed the client, and then verified and the perms were correct. Maybe I was dreaming though!

chrisschaub’s picture

Ok, this still needs work. It needs to be recursive to really work as mentioned in #6 by mvc. Not sure the best way to do this other than set the recursive = true.

anarcat’s picture

Status: Active » Fixed

I am giving up and just setting ACLs on the whole directory, recursively. As a basic safety measure, i make the drushrc and settings.php read-only, but that's only indicative: a user could remove and recreate those files since they can write to the directory.

Besides, now that they have write access to the directory, they can just write to local.settings.php.

Basically, this means that we broadened the attack surface for the security issue described in #762138: Design security issue with developer access to sites' modules and themes.

But I think it's worth it right now because the security issue exists regardless of how provisionacl behaves. It just makes it a bit harder to fix because we can bootstrap even less than before.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.