made actions.py more pep8 compliant; added one more check to verify that an action is within the actions directory; moved actions directory path into the settings instead of hard-coding it

actions.py

@@ -73,46 +73,44 @@ Actions run commands with this contract (version 1.1):
 """
 
 import os
-import pipes, shlex, subprocess
+import pipes
+import subprocess
+
+import cfg
 
 
-def run(action, options = None, async = False):
+def run(action, options=None, async=False):
     """Safely run a specific action as the current user.
-
     See actions._run for more information.
-
     """
     return _run(action, options, async, False)
 
-def superuser_run(action, options = None, async = False):
+
+def superuser_run(action, options=None, async=False):
     """Safely run a specific action as root.
-
     See actions._run for more information.
-
     """
     return _run(action, options, async, True)
 
-def _run(action, options = None, async = False, run_as_root = False):
+
+def _run(action, options=None, async=False, run_as_root=False):
     """Safely run a specific action as a normal user or root.
-
     actions are pulled from the actions directory.
-
+    - options are added to the action command.
-    options are added to the action command.
+    - async: run asynchronously or wait for the command to complete.
-
+    - run_as_root: execute the command through sudo.
-    async: run asynchronously or wait for the command to complete.
-
-    run_as_root: execute the command through sudo.
     """
-    DIRECTORY = "actions"
 
-    if options == None:
+    if options is None:
         options = []
 
     # contract 3A and 3B: don't call anything outside of the actions directory.
     if os.sep in action:
         raise ValueError("Action can't contain:" + os.sep)
 
-    cmd = DIRECTORY + os.sep + action
+    cmd = cfg.actions_dir + os.sep + action
+    if not os.path.realpath(cmd).startswith(cfg.actions_dir):
+        raise ValueError("Action has to be in directory %s" % cfg.actions_dir)
 
     # contract 3C: interpret shell escape sequences as literal file names.
     # contract 3E: fail if the action doesn't exist or exists elsewhere.
@@ -125,7 +123,6 @@ def _run(action, options = None, async = False, run_as_root = False):
     if options:
         if not hasattr(options, "__iter__"):
             options = [options]
-
         cmd += [pipes.quote(option) for option in options]
 
     # contract 1: commands can run via sudo.
@@ -136,8 +133,8 @@ def _run(action, options = None, async = False, run_as_root = False):
     # contract 5 (and 6-ish).
     proc = subprocess.Popen(
         cmd,
-        stdout = subprocess.PIPE,
+        stdout=subprocess.PIPE,
-        stderr= subprocess.PIPE,
+        stderr=subprocess.PIPE,
         shell=False)
 
     if not async:

cfg.py

@@ -41,6 +41,7 @@ def read():
                     ('Path', 'data_dir'),
                     ('Path', 'store_file'),
                     ('Path', 'user_db'),
+                    ('Path', 'actions_dir'),
                     ('Path', 'status_log_file'),
                     ('Path', 'access_log_file'),
                     ('Path', 'pidfile'),

plinth.sample.config

@@ -10,6 +10,7 @@ log_dir = %(data_dir)s
 pid_dir = %(data_dir)s
 python_root = %(file_root)s
 server_dir = plinth/
+actions_dir = %(file_root)s/actions
 
 # file locations
 store_file = %(data_dir)s/store.sqlite3