Merge remote-tracking branch 'origin/pr/176'

* origin/pr/176:
  Provide clear error on invalid policy file name

Author: marmarek

qrexec/policy/admin.py

@@ -24,6 +24,7 @@
 import contextlib
 import fcntl
 import os
+import re
 import hashlib
 
 from .parser import ValidateParser, FilePolicy, get_invalid_characters
@@ -87,7 +88,11 @@ def handle_request(
         Throws PolicyAdminException in case of user error.
         """
 
-        assert all(char in RPCNAME_ALLOWED_CHARSET for char in arg)
+        if not all(char in RPCNAME_ALLOWED_CHARSET for char in arg):
+            raise PolicyAdminException(
+                "Invalid argument: \"{}\"\n"
+                "Valid characters are letters, numbers, dot, plus, hyphen and "
+                "underline".format(arg))
 
         func = self._find_method(service_name)
         if not func:
@@ -266,7 +271,12 @@ def policy_get_files(self, arg):
 
     # helpers
 
-    def _get_path(self, arg: str, dir_path, suffix: str) -> Path:
+    def _get_path(self, arg: str, dir_path: str , suffix: str) -> Path:
+        if not re.compile(r'^[\w-]+$').match(arg):
+            raise PolicyAdminException(
+                f"Invalid policy file name: {arg}\n"
+                "Names must contain only alphanumeric characters, "
+                "underscore and hyphen.")
         path = dir_path / (arg + suffix)
         path = path.resolve()
         if path.parent != dir_path:

qrexec/tests/policy_admin.py

@@ -76,12 +76,18 @@ def test_api_get(policy_dir, api):
     with pytest.raises(PolicyAdminException, match="Not found"):
         api.handle_request("policy.Get", "nonexistent", b"")
 
-    with pytest.raises(PolicyAdminException, match="Expecting a path inside"):
+    with pytest.raises(PolicyAdminException, match="Invalid policy file"):
+        api.handle_request("policy.Get", ".hidden_evil_policy", b"")
+
+    with pytest.raises(PolicyAdminException, match="Invalid policy file"):
         api.handle_request("policy.include.Get", "..", b"")
 
-    with pytest.raises(PolicyAdminException, match="Expecting a path inside"):
+    with pytest.raises(PolicyAdminException, match="Invalid policy file"):
         api.handle_request("policy.include.Get", "", b"")
 
+    with pytest.raises(PolicyAdminException, match="Invalid argument"):
+        api.handle_request("policy.include.Get", "space in argument", b"")
+
 
 def test_api_replace(policy_dir, api):
     api.handle_request("policy.Replace", "file1", b"any\n")