Add PHP execution permissions

hm, yes technically they don't have to be connected. I'm not sure whether it makes sense for users though, so users not confident with PHP can just disable the PHP module and it's away in rules too.

So yes, perhaps it would have been better to make this an separate option or module. But changing it now, would be
* quite some work as we would need an update procedure
* and complicated, as integration as the cck integration relys on it.

So changing it, would require changes to the cck integration, and cause version compatibility issues between the two. So I think we better leave it like it's now.. ;)

Another use case is when you want to have the PHP module enabled, but only allow certain roles to use PHP evaluation in rules. This is with regards to security. E.g. a "superadmin" role may use the PHP input filter and PHP in blocks and PHP evaluation in rules, while the "admin" role should not be allowed to use PHP at all while still being able to administer rules (only without PHP evaluation). Adding something like a "use php evaluation" permission to your module would be great!

I suppose this would be a bit complicated to fix, but I think it's something you should consider, as this type of roles setup is common on many larger sites. Until it is fixed, is there something we can do to achieve what we want?

Would it be possible to do the following?

1. Add permission 'use php evaluation' to Rules module itself if PHP module is enabled
2. Add '#access' = (module_exists('php') && user_access('use php evaluation')) to all PHP Evaluation form elements

There are no separate form elements for php evaluation, so it's not so simple. This would require significant changes and won't happen for 1.x.

I feared it wasn't that simple :)

We did the following in a custom module (function names changed here for convenience) to achieve what we wanted...

/**
 * Implementation of hook_perm().
 */
function custom_perm() {
  return array('use php evaluator in rules');
}

/**
 * Implementation of hook_form_alter().
 */
function custom_form_alter(&$form, $form_state, $form_id) {
  if (in_array($form_id, array('rules_admin_form_add', 'rules_admin_form_edit'))
      && !user_access('use php evaluation')) {
        if (isset($form['name']['#options']['PHP'])) {
          unset($form['name']['#options']['PHP']);
        }
        $form['advanced_options']['#access'] = false;
        $form['input_help']['rules_input_evaluator_php']['#access'] = false;
        $form['submit']['#validate'][] = '_custom_rules_admin_form_validate';
  }
}

/**
 * Custom validation callback for rules_admin_form_add/edit forms.
 */
function _custom_rules_admin_form_validate($form, &$form_state) {
  if (is_array($form_state['values']['settings'])) {
    foreach ($form_state['values']['settings'] as $key => $element) {
      if (strpos($form_state['values']['settings'][$key], '<?') !== false) {
        $form_state['values']['settings'][$key] = preg_replace('/<\?.*\?>/is', '', $form_state['values']['settings'][$key]);
        drupal_set_message(t('Harmful PHP code was removed.'));
      }
    }
  }
}

If there are any other places where PHP may be used in Rules, I'd be happy to hear about it.

Still, I think this is a big drawback to the (otherwise awesome!) Rules module, and it would be really great if this could be fixed in 1.x.

@ximo: that sounds like a self-assignment to me :)

I think this could potentially be an important security measure for our various rules ui implementations, in addition to cck text areas.

>drupal_set_message(t('Harmful PHP code was removed.'));

S if a not-privileged user edits a rule containg PHP code, it code would be removed? So those not-privileged user can is able to damage those rules? - that's a serious problem..

To be able to implement this properly, we would need access checks built in into rules, so that those users can't even edit a rule containing PHP code they may not access. Already just adding or changing a condition of a rule, which contains PHP code in the action, could have serious consequences.

@fago:
Very true. In our case, the rules didn't have any PHP to begin with, so our little module only served as a workaround, making sure no PHP code was submitted. Having a permission for editing rules containing PHP would be much better. Unfortunately, I'v got too many commitments at the moment to write a patch implementing that. I wanted to raise awareness of this security issue at least.

@mitchell:
It wasn't intended to :) And I agree, with a decoupled UI, this is an important security issue. Btw, CCK has the "Use PHP input for field settings (dangerous - grant with care)" permission.

Tags: cron

I think this is the reason for my problem. I use rules and have written some custom php snippets and set this to run via cron. When I run cron, logged in as admin, it all works. Job done I think - but it seems not. Cron runs as anonymous and so no permission to use php so my rule doesn't run! The log to watchdog at the bottom works so when I check my logs it says all is fine, but nothing is happening. I think php execution in rules is very untrustworthy. It shouldn't be related to the general php permission it should either do it not. Do users write rules? I would have thought this is strictly admin.

It seems like I need to go back and change all my rules, most of which use a bit of php. Or is this not my problem? I can't understand why this only seems to affect cron. I would have thought my other rules would have given noticeable errors too. Perhaps this is something to do with cron not loading everything needed to run php?

UPDATE: My problems would seem to be views. Even though views has its own access restrictions setting on a per view basis it looks to be obeying the access restriction of the nodes too. So if anonymous users can't see the node then cron can't get info from the view such as a list of nodes to expire.

The aim of rules 2.x is to make php eval unnecessary, so working on that instead of improving PHP is preferred. Still rules 2.x already applies the permission of the PHP module.

Updated component.