Repository: aiidalab/aiidalab-launch (language: python)

Issue:
Restrict or normalize the profile name
## Problem

The profile name is used to generate the name for the associated docker container. Therefore, using profile names with invalid characters (e.g., `:`) or too long names will lead to problems.

## Proposed solution

Check the profile name, restrict its length and ensure it is url-safe.


Discussion / hints:

Fix patch:
diff --git a/aiidalab_launch/__main__.py b/aiidalab_launch/__main__.py
index c531621..c365f95 100644
--- a/aiidalab_launch/__main__.py
+++ b/aiidalab_launch/__main__.py
@@ -178,7 +178,11 @@ def add_profile(ctx, app_state, profile):
     else:
         raise click.ClickException(f"Profile with name '{profile}' already exists.")
 
-    new_profile = Profile(name=profile)
+    try:
+        new_profile = Profile(name=profile)
+    except ValueError as error:  # invalid profile name
+        raise click.ClickException(error)
+
     app_state.config.profiles.append(new_profile)
     app_state.config.save(_application_config_path())
     click.echo(f"Added profile '{profile}'.")
diff --git a/aiidalab_launch/core.py b/aiidalab_launch/core.py
index 76a454f..c0105c9 100644
--- a/aiidalab_launch/core.py
+++ b/aiidalab_launch/core.py
@@ -5,12 +5,14 @@
 
 import asyncio
 import logging
+import re
 from contextlib import contextmanager
 from dataclasses import asdict, dataclass, field
 from enum import Enum, auto
 from pathlib import Path
 from secrets import token_hex
 from typing import Any, AsyncGenerator, Generator, List, Optional, Union
+from urllib.parse import quote_plus
 from uuid import uuid4
 
 import docker
@@ -28,6 +30,12 @@
 
 LOGGER = logging.getLogger(APPLICATION_ID.split(".")[-1])
 
+# Regular expression for valid container names as of Docker version 20.10.11:
+# [a-zA-Z0-9][a-zA-Z0-9_.-]
+# In addition we do not allow underscores to avoid potential issues with moving
+# to a docker-compose based implementation of aiidalab-launch in the future.
+_REGEX_VALID_PROFILE_NAMES = r"[a-zA-Z0-9][a-zA-Z0-9.-]+"
+
 
 def _default_home_mount() -> str:
     return str(Path.home().joinpath("aiidalab"))
@@ -46,6 +54,17 @@ class Profile:
     image: str = "aiidalab/aiidalab-docker-stack:latest"
     home_mount: Optional[str] = field(default_factory=lambda: _default_home_mount())
 
+    def __post_init__(self):
+        if (
+            not re.fullmatch(_REGEX_VALID_PROFILE_NAMES, self.name)
+            or quote_plus(self.name) != self.name
+        ):
+            raise ValueError(
+                f"Invalid profile name '{self.name}'. The profile name must be "
+                "composed of the following characters [a-zA-Z0-9.-] and must "
+                "start with an alphanumeric character."
+            )
+
     def container_name(self) -> str:
         return f"{CONTAINER_PREFIX}{self.name}"
 


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 72ebfe9..1bf8dcf 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -54,15 +54,15 @@ def test_show_profile():
 
 def test_add_remove_profile():
     runner: CliRunner = CliRunner()
-    result: Result = runner.invoke(cli.cli, ["profiles", "add", "new_profile"])
-    assert "Added profile 'new_profile'." in result.output
+    result: Result = runner.invoke(cli.cli, ["profiles", "add", "new-profile"])
+    assert "Added profile 'new-profile'." in result.output
     result: Result = runner.invoke(cli.cli, ["profiles", "list"])
-    assert "new_profile" in result.output
-    result: Result = runner.invoke(cli.cli, ["profiles", "show", "new_profile"])
+    assert "new-profile" in result.output
+    result: Result = runner.invoke(cli.cli, ["profiles", "show", "new-profile"])
     assert result.exit_code == 0
     result: Result = runner.invoke(
-        cli.cli, ["profiles", "remove", "new_profile"], input="y\n"
+        cli.cli, ["profiles", "remove", "new-profile"], input="y\n"
     )
     assert result.exit_code == 0
     result: Result = runner.invoke(cli.cli, ["profiles", "list"])
-    assert "new_profile" not in result.output
+    assert "new-profile" not in result.output
diff --git a/tests/test_core.py b/tests/test_core.py
index dc872d2..17854a5 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -14,6 +14,10 @@
 
 from aiidalab_launch.core import AiidaLabInstance, Config, Profile
 
+VALID_PROFILE_NAMES = ["abc", "Abc", "aBC", "a0", "a-a", "a-0"]
+
+INVALID_PROFILE_NAMES = ["", ".a", "a_a", "_a"]
+
 
 @pytest.fixture
 def profile():
@@ -42,6 +46,17 @@ def test_profile_init(profile):
     pass
 
 
+@pytest.mark.parametrize("name", VALID_PROFILE_NAMES)
+def test_profile_init_valid_names(profile, name):
+    assert replace(profile, name=name).name == name
+
+
+@pytest.mark.parametrize("name", INVALID_PROFILE_NAMES)
+def test_profile_init_invalid_names(profile, name):
+    with pytest.raises(ValueError):
+        replace(profile, name=name)
+
+
 def test_profile_equality(profile):
     assert profile == profile
     assert profile != replace(profile, name="other")