Validate environment paths

softpack_core/schemas/environment.py

@@ -158,11 +158,24 @@
             return InvalidInputError(message="all fields must be filled in")
 
         if not re.fullmatch(r"^[a-zA-Z0-9_-]+$", self.name):
             return InvalidInputError(
                 message="name must only contain alphanumerics, "
                 "dash, and underscore"
             )
 
+        valid_dirs = [
+            Artifacts.users_folder_name,
+            Artifacts.groups_folder_name,
+        ]
+        if not any(self.path.startswith(dir + "/") for dir in valid_dirs):
+            return InvalidInputError(message="Invalid path")
+
+        if not re.fullmatch(r"^[^/]+/[a-zA-Z0-9_-]+$", self.path):
+            return InvalidInputError(
+                message="user/group subdirectory must only contain "
+                "alphanumerics, dash, and underscore"
+            )
+
         return None
 
     @classmethod
@@ -182,8 +195,8 @@
         return EnvironmentInput(
             name=environment_name,
             path="/".join(environment_dirs),
-            description="",
-            packages=list(),
+            description="placeholder description",
+            packages=[PackageInput("placeholder")],
         )
 
 
@@ -247,15 +260,16 @@
         Returns:
             A message confirming the success or failure of the operation.
         """
-        input_err = env.validate()
-        if input_err is not None:
-            return input_err
-
         versionless_name = env.name
         version = 1
 
+        if not versionless_name:
+            return InvalidInputError(
+                message="environment name must not be blank"
+            )
+
         while True:
             env.name = versionless_name + "-" + str(version)
             response = cls.create_new_env(
                 env, Artifacts.built_by_softpack_file
             )
@@ -268,9 +282,9 @@
             version += 1
 
         # Send build request
         try:
             host = app.settings.builder.host
             port = app.settings.builder.port
             r = httpx.post(
                 f"http://{host}:{port}/environments/build",
                 json={
@@ -288,10 +302,10 @@
                     },
                 },
             )
             r.raise_for_status()
         except Exception as e:
             cls.delete(env.name, env.path)
             return BuilderError(
                 message="Connection to builder failed: "
                 + "".join(format_exception_only(type(e), e))
             )
@@ -319,15 +333,12 @@
             CreateResponse: a CreateEnvironmentSuccess on success, or one of
             (InvalidInputError, EnvironmentAlreadyExistsError) on error.
         """
-        # Check if a valid path has been provided. TODO: improve this to check
-        # that they can only create stuff in their own users folder, or in
+        # TODO: improve this to check
+        # that users can only create stuff in their own users folder, or in
         # group folders of unix groups they belong to.
-        valid_dirs = [
-            cls.artifacts.users_folder_name,
-            cls.artifacts.groups_folder_name,
-        ]
-        if not any(env.path.startswith(dir) for dir in valid_dirs):
-            return InvalidInputError(message="Invalid path")
+        input_err = env.validate()
+        if input_err is not None:
+            return input_err
 
         # Check if an env with same name already exists at given path
         if cls.artifacts.get(Path(env.path), env.name):
@@ -347,9 +358,9 @@
                     for pkg in env.packages
                 ],
             )
             ymlData = yaml.dump(softpack_definition)
 
             tree_oid = cls.artifacts.create_files(
                 new_folder_path,
                 [
                     (env_type, ""),  # e.g. .built_by_softpack
@@ -361,7 +372,7 @@
                 tree_oid, "create environment folder"
             )
         except RuntimeError as e:
             return InvalidInputError(
                 message="".join(format_exception_only(type(e), e))
             )
 
@@ -572,13 +583,13 @@
             tree_oid = cls.artifacts.create_files(
                 Path(folder_path), new_files, overwrite=True
             )
             cls.artifacts.commit_and_push(tree_oid, "write artifact")
             return WriteArtifactSuccess(
                 message="Successfully written artifact(s)",
             )
 
         except Exception as e:
             return InvalidInputError(
                 message="".join(format_exception_only(type(e), e))
             )
 

tests/integration/test_environment.py

@@ -124,8 +124,19 @@ def test_create_name_dashes_and_number_first_allowed(
     assert isinstance(result, CreateEnvironmentSuccess)
 
 
-def test_create_path_invalid_disallowed(httpx_post, testable_env_input):
-    testable_env_input.path = "invalid/path"
+@pytest.mark.parametrize(
+    "path",
+    [
+        "invalid/path",
+        "users/",
+        "users/username/foo",
+        "users/abc123/very/nested",
+        "users/../nasty",
+        "users/user name",
+    ],
+)
+def test_create_path_invalid_disallowed(httpx_post, testable_env_input, path):
+    testable_env_input.path = path
     result = Environment.create(testable_env_input)
     assert isinstance(result, InvalidInputError)