refactor: use regex pattern for SUPERVISOR_ env var validation

- Replace manual string parsing with compiled regex pattern
- Explicitly validate section/key format (no leading/trailing underscores)
- Add comprehensive test coverage for edge cases
- Improve code maintainability and clarity

Author: HappyAmazonian

python/model_hosting_container_standards/supervisor/models.py

@@ -118,55 +118,42 @@ def _parse_supervisor_custom_sections() -> Dict[str, Dict[str, str]]:
     Returns:
         Dictionary mapping section names to their key-value configurations
     """
+    import re
+
+    # Pattern matches SUPERVISOR_SECTION_KEY where:
+    # - SECTION: alphanumeric, may contain __ (for colons) or _ (internal), no leading/trailing _
+    # - KEY: alphanumeric, may contain _ (internal), no leading/trailing _
+    pattern = re.compile(
+        r"^SUPERVISOR_"
+        r"(?P<section>[A-Z0-9]+(?:__[A-Z0-9]+|_[A-Z0-9]+)*)"  # SECTION (__ for colons)
+        r"_(?P<key>[A-Z0-9]+(?:_[A-Z0-9]+)*)$"  # KEY (no leading/trailing _)
+    )
+
     custom_sections: Dict[str, Dict[str, str]] = {}
 
     for env_var, value in os.environ.items():
-        if not env_var.startswith("SUPERVISOR_"):
-            continue
-
-        # Skip the config path variable
+        # Skip non-SUPERVISOR_ variables and the config path variable
         if env_var == "SUPERVISOR_CONFIG_PATH":
             continue
 
-        # Remove SUPERVISOR_ prefix
-        remaining = env_var[11:]  # len("SUPERVISOR_") = 11
-
-        # Find the last underscore to separate key from section
-        last_underscore = remaining.rfind("_")
-        if last_underscore == -1:
-            logger.warning(
-                f"Invalid SUPERVISOR_ environment variable format: '{env_var}'. "
-                f"Expected format: SUPERVISOR_SECTION_KEY=value"
-            )
+        match = pattern.match(env_var)
+        if not match:
+            # Only warn if it starts with SUPERVISOR_ but doesn't match pattern
+            if env_var.startswith("SUPERVISOR_"):
+                logger.warning(
+                    f"Invalid SUPERVISOR_ environment variable format: '{env_var}'. "
+                    f"Expected format: SUPERVISOR_SECTION_KEY=value (alphanumeric with underscores, "
+                    f"no leading/trailing underscores, use __ for section colons)"
+                )
             continue
 
-        section_part = remaining[:last_underscore]
-        key_name = remaining[last_underscore + 1 :].lower()
+        # Extract section and key from regex groups
+        section_part = match.group("section")
+        key_name = match.group("key").lower()
 
-        # Convert double underscores to colons in section name first
+        # Convert double underscores to colons in section name
         section_name = section_part.replace("__", ":").lower()
 
-        # Validate section and key are not empty after processing
-        # Also check for invalid section names (starting with underscore indicates empty section before __)
-        if (
-            not section_name
-            or section_name.startswith(":")
-            or section_name.endswith(":")
-            or section_name.startswith("_")
-        ):
-            logger.warning(
-                f"Invalid SUPERVISOR_ environment variable: '{env_var}' has invalid section name. "
-                f"Expected format: SUPERVISOR_SECTION_KEY=value"
-            )
-            continue
-
-        if not key_name:
-            logger.warning(
-                f"Invalid SUPERVISOR_ environment variable: '{env_var}' has empty key name. "
-                f"Expected format: SUPERVISOR_SECTION_KEY=value"
-            )
-            continue
-
         # Initialize section if it doesn't exist
         if section_name not in custom_sections:
             custom_sections[section_name] = {}

python/tests/supervisor/test_models.py

@@ -294,6 +294,86 @@ def test_invalid_format_ignored(self):
             # All invalid formats should be ignored, result should be empty
             assert result == {}
 
+    def test_leading_underscore_in_section_rejected(self):
+        """Test that section names with leading underscores are rejected."""
+        test_env = {
+            "SUPERVISOR__PROGRAM_COMMAND": "python app.py",  # Leading underscore in section
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+            assert result == {}
+
+    def test_trailing_underscore_in_section_rejected(self):
+        """Test that section names with trailing underscores are rejected."""
+        test_env = {
+            "SUPERVISOR_PROGRAM__COMMAND": "python app.py",  # Trailing underscore in section (before key)
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+            assert result == {}
+
+    def test_multiple_consecutive_underscores_rejected(self):
+        """Test that three or more consecutive underscores are rejected."""
+        test_env = {
+            "SUPERVISOR_PROGRAM___WEB_COMMAND": "gunicorn app:app",  # Three underscores
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+            assert result == {}
+
+    def test_leading_underscore_in_key_rejected(self):
+        """Test that key names with leading underscores are rejected."""
+        test_env = {
+            "SUPERVISOR_PROGRAM__COMMAND": "python app.py",  # Leading underscore in key
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+            assert result == {}
+
+    def test_trailing_underscore_in_key_rejected(self):
+        """Test that key names with trailing underscores are rejected."""
+        test_env = {
+            "SUPERVISOR_PROGRAM_COMMAND_": "python app.py",  # Trailing underscore in key
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+            assert result == {}
+
+    def test_numeric_only_sections_and_keys_accepted(self):
+        """Test that purely numeric section and key names are accepted."""
+        test_env = {
+            "SUPERVISOR_123_456": "value",  # Numeric section and key
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+
+            expected = {
+                "123": {"456": "value"},
+            }
+            assert result == expected
+
+    def test_mixed_alphanumeric_accepted(self):
+        """Test that mixed alphanumeric section and key names are accepted."""
+        test_env = {
+            "SUPERVISOR_PROGRAM2_COMMAND3": "python app.py",
+            "SUPERVISOR_WEB1__API2_PORT8080": "8080",
+        }
+
+        with patch.dict(os.environ, test_env, clear=True):
+            result = _parse_supervisor_custom_sections()
+
+            expected = {
+                "program2": {"command3": "python app.py"},
+                "web1:api2": {"port8080": "8080"},
+            }
+            assert result == expected
+
 
 class TestParseEnvironmentVariables:
     """Test the main parse_environment_variables function."""