review permissions, set "restrict access" where appropriate, make "generate feature" require special permission [#2060665]

Someone mentioned to me that features 2.0 lets you "generate" the feature right onto the working directory of the site. I understand this feature is primarily for "development" purposes but it's not labeled that way. That got me looking into Features permissions and I'd like to propose a few changes there.

1. Right now, none of the permissions have "restrict access" set to true, but my sense is that both admin features and managing features probably should be somewhat restricted in who can do them. Allowing a non-admin to disable a feature that contains the front-page feed, for example, would be a real bummer.
2. Right now it's possible to use the generate feature code to put code into the live site. Granted it can only contain the variables available on the site, but this ability to upload code to working site feels like it opens a risky slope and shouldn't be enabled on sites that are paranoid. So...I suggest making that feature available only with its own permission.

Comment	File	Size	Author
#3	features-restrict-permissions-2060665.patch	3.06 KB	mpotter
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

hefox CreditAttribution: hefox commented 9 August 2013 at 22:56

Priority:

Normal

» Critical

I was just thinking about this a few minutes ago o.o.

I think this is a stable release blocker type thing, so putting it as critical for now.

I haven't looked at the generate code at all or documentation for it; if it's not documented already that this should not be used on a live site, then likely that documentation should be added as well. Perhaps as part of the new permission.

Hm, should creating features be a seperate one from managing, if it's not already?

Comment #2

greggles

he/him

English

Denver, Colorado, USA

CreditAttribution: greggles commented 14 August 2013 at 14:09

Agreed on the priority. I also see this as a release blocker but didn't want to be presumptuous in someone else's queue.

Greg

Comment #3

mpotter CreditAttribution: mpotter commented 14 August 2013 at 14:51

Status:

Active

» Needs review

File	Size
features-restrict-permissions-2060665.patch	3.06 KB

Here is an initial stab at this. I added 'restrict access' to the permissions. That's definitely a no-brainer.

For the Generate Feature, I added a new permission and then use it to prevent the button from being generated, but also prevent generation directly in features_export_build_form_submit() from happening in case somebody tries to inject stuff into the form.

If somebody wants to add some documentation for warning about Generate, that would be welcome. In general though, production sites shouldn't have the file permissions open to allow the web server to write/update files to the sites/all/modules directory, so I don't think this is a huge problem. It *does* allow you to change the path and write the export to something like /tmp and then you can log in via ssh and move the code. But that's pretty much the same as what "drush fu" would allow you to do.

Comment #4

greggles

he/him

English

Denver, Colorado, USA

CreditAttribution: greggles commented 14 August 2013 at 19:57

In general though, production sites shouldn't have the file permissions open to allow the web server to write/update files to the sites/all/modules directory, so I don't think this is a huge problem.

Yeah, so the weird thing to me is that someone who is fairly security conscious said that they used Features2 to generate code and that they wanted security_review.module to ignore that directory as being safe. I had to explain it wasn't safe :)

That feels like a separate issue - I'll file that now.