From 0539dfb28a67b41fc673875d9ca146a94741f956 Mon Sep 17 00:00:00 2001
From: Nick Daly <Nick.M.Daly@gmail.com>
Date: Sun, 17 Nov 2013 16:01:12 -0600
Subject: [PATCH] Privileged Actions can take option lists again.

---
 privilegedactions.py            | 19 +++++++++----------
 tests/privelegedactions_test.py |  7 +++----
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/privilegedactions.py b/privilegedactions.py
index e26dfe44a..f42c22c53 100644
--- a/privilegedactions.py
+++ b/privilegedactions.py
@@ -50,6 +50,10 @@ Privileged actions run commands with this contract (version 1.0):
       ValueError.  They don't because sanitizing this case is significantly
       easier than detecting if it occurs.
 
+      The options list is coerced into a space-separated string before being
+      shell-escaped.  Option lists including shell escape characters may need to
+      be unescaped on the receiving end.
+
    E. Actions must exist in the actions directory.
 
 4. (promise) Options are appended to the action.
@@ -86,12 +90,6 @@ def privilegedaction_run(action, options = None):
     # 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)
-    if not isinstance(options, str):
-        raise ValueError("Options must be a string and cannot be a list.")
-
-    # contract: 3C, 3D: don't allow users to insert escape characters.
-    action = pipes.quote(action)
-    options = pipes.quote(options)
 
     cmd = DIRECTORY + os.sep + action
 
@@ -100,11 +98,11 @@ def privilegedaction_run(action, options = None):
     if not os.access(cmd, os.F_OK):
         raise ValueError("Action must exist in action directory.")
 
-    cmd = ["sudo", "-n", cmd]
+    if hasattr(options, "__iter__"):
+        options = " ".join(options)
 
-    # contract 4.
-    if options:
-        cmd.extend(shlex.split(options))
+    # contract: 3C, 3D: don't allow users to insert escape characters
+    cmd = ["sudo", "-n", cmd, pipes.quote(options)]
 
     # contract 3C: don't interpret shell escape sequences.
     # contract 5 (and 6-ish).
@@ -113,4 +111,5 @@ def privilegedaction_run(action, options = None):
                          stdout = subprocess.PIPE,
                          stderr= subprocess.PIPE,
                          shell=False).communicate()
+
     return output, error
diff --git a/tests/privelegedactions_test.py b/tests/privelegedactions_test.py
index 8ba646da3..aa88e9499 100644
--- a/tests/privelegedactions_test.py
+++ b/tests/privelegedactions_test.py
@@ -109,11 +109,10 @@ class TestPrivileged(unittest.TestCase):
         # counting is safer than actual badness.
         options = ["good", ";", "echo $((1+1))"]
 
-        with self.assertRaises(ValueError):
-            output, error = privilegedaction_run(action, options)
+        output, error = privilegedaction_run(action, options)
 
-            # if it somehow doesn't error, we'd better not evaluate the data.
-            self.assertFalse("2" in output)
+        # we'd better not evaluate the data.
+        self.assertFalse("2" in output)
 
 if __name__ == "__main__":
     unittest.main()