feat: Check malformed path entries

_appmap/__init__.py

@@ -1,6 +1,5 @@
-from . import configuration
+from . import configuration, event, importer, metadata, recorder, recording, web_framework
 from . import env as appmapenv
-from . import event, importer, metadata, recorder, recording, web_framework
 from .py_version_check import check_py_version
 
 

_appmap/configuration.py

@@ -2,6 +2,7 @@
 Manage Configuration AppMap recorder for Python.
 """
 
+import ast
 import importlib.metadata
 import inspect
 import os
@@ -14,6 +15,7 @@
 from yaml.parser import ParserError
 
 from _appmap.labels import LabelSet
+from _appmap.singleton import SingletonMeta
 from appmap.labeling import presets as label_presets
 
 from . import utils
@@ -124,25 +126,13 @@ def excluded(d):
 
     return packages
 
+class AppMapInvalidConfigException(Exception):
+    pass
 
-class Config:
+class Config(metaclass=SingletonMeta):
     """Singleton Config class"""
 
-    _instance = None
-
-    def __new__(cls):
-        if cls._instance is None:
-            logger.trace("Creating the Config object")
-            cls._instance = super(Config, cls).__new__(cls)
-
-            cls._instance._initialized = False
-
-        return cls._instance
-
     def __init__(self):
-        if self._initialized:
-            return
-
         self.file_present = False
         self.file_valid = False
         self.package_functions = {}
@@ -154,12 +144,6 @@ def __init__(self):
         if "labels" in self._config:
             self.labels.append(self._config["labels"])
 
-        self._initialized = True
-
-    @classmethod
-    def initialize(cls):
-        cls._instance = None
-
     @property
     def name(self):
         return self._config["name"]
@@ -198,7 +182,7 @@ def default_packages(self):
         root_dir = Env.current.root_dir
         return [{"path": p} for p in find_top_packages(root_dir)]
 
-    def _load_config(self):
+    def _load_config(self, show_warnings=False):
         self._config = {"name": None, "packages": []}
 
         # Only use a default config if the user hasn't specified a
@@ -211,7 +195,7 @@ def _load_config(self):
 
         env = Env.current
         config_dir = env.root_dir
-        
+
         path = _resolve_relative_to(Path(env_config_filename), Path(config_dir))
         if not path.is_file():
             # search config file in parent directories up to
@@ -240,6 +224,8 @@ def _load_config(self):
                         self._config["name"] = self.default_name
                     if "packages" not in self._config:
                         self._config["packages"] = self.default_packages
+                    else:
+                        self._drop_malformed_package_paths(show_warnings)
 
                     # Is appmap_dir specified?
                     appmap_dir = (
@@ -303,6 +289,43 @@ def _load_functions(self):
 
         self.package_functions.update(modules)
 
+    def _drop_malformed_package_paths(self, show_warnings):
+        invalid_items = []
+        for item in self._config["packages"]:
+            # it can be a "dist" entry
+            if "path" not in item:
+                continue
+
+            path = item.get("path")
+            if path is None:
+                if show_warnings:
+                    logger.warning("Missing path value in configuration file.")
+                invalid_items.append(item)
+                continue
+
+            if not self._check_path_value(path):
+                has_separator = isinstance(path, str) and ('/' in path or '\\' in path)
+                if show_warnings:
+                    logger.warning(
+                        f"Malformed path value '{path}' in configuration file. "
+                        "Path entries must be module names"
+                        f"{' not directory paths' if has_separator else ''}.",
+                        stack_info=False,
+                    )
+                invalid_items.append(item)
+                continue
+
+        if len(invalid_items) > 0:
+            self._config["packages"] = [item for item in self._config["packages"]
+                                        if item not in invalid_items]
+
+    def _check_path_value(self, value):
+        try:
+            ast.parse(f"import {value}")
+            return True
+        except SyntaxError:
+            return False
+
 
 def startswith(prefix, sequence):
     """
@@ -395,7 +418,7 @@ def wrap(self, filterable):
         wrapped = getattr(filterable.obj, "_appmap_wrapped", None)
         if wrapped is None:
             logger.trace("  wrapping %s", filterable.fqname)
-            Config().labels.apply(filterable)
+            Config.current.labels.apply(filterable)
             ret = instrument(filterable)
             if rule and rule.shallow:
                 setattr(ret, "_appmap_shallow", rule)
@@ -428,7 +451,7 @@ class ConfigFilter(MatcherFilter):
     def __init__(self, *args, **kwargs):
         matchers = []
         if Env.current.enabled:
-            matchers = [matcher_of_config(p) for p in Config().packages]
+            matchers = [matcher_of_config(p) for p in Config.current.packages]
         super().__init__(matchers, *args, **kwargs)
 
 
@@ -441,14 +464,15 @@ def __init__(self, *args, **kwargs):
 
 
 def initialize():
-    Config().initialize()
+    Config.reset()
     Importer.use_filter(BuiltinFilter)
     Importer.use_filter(ConfigFilter)
 
 
 initialize()
 
-c = Config()
+c = Config.current
+c._load_config(show_warnings=True)
 logger.info("config: %s", c._config)
 logger.debug("package_functions: %s", c.package_functions)
 logger.info("env: %r", os.environ)

_appmap/env.py

@@ -10,6 +10,8 @@
 from pathlib import Path
 from typing import cast
 
+from _appmap.singleton import SingletonMeta
+
 from . import trace_logger
 
 _ENABLED_BY_DEFAULT_MSG = """
@@ -34,23 +36,7 @@ def _recording_method_key(recording_method):
     return f"APPMAP_RECORD_{recording_method.upper()}"
 
 
-class _EnvMeta(type):
-    def __init__(cls, *args, **kwargs):
-        type.__init__(cls, *args, **kwargs)
-        cls._instance = None
-
-    @property
-    def current(cls):
-        if not cls._instance:
-            cls._instance = Env()
-
-        return cls._instance
-
-    def reset(cls, **kwargs):
-        cls._instance = Env(**kwargs)
-
-
-class Env(metaclass=_EnvMeta):
+class Env(metaclass=SingletonMeta):
     def __init__(self, env=None, cwd=None):
         warnings.filterwarnings("once", _ENABLED_BY_DEFAULT_MSG)
 

_appmap/importer.py

@@ -189,7 +189,7 @@ def instrument_functions(filterable, selected_functions=None):
         # Import Config here, to avoid circular top-level imports.
         from .configuration import Config  # pylint: disable=import-outside-toplevel
 
-        package_functions = Config().package_functions
+        package_functions = Config.current.package_functions
         fm = FilterableMod(mod)
         if fm.fqname in package_functions:
             instrument_functions(fm, package_functions.get(fm.fqname))

_appmap/singleton.py

@@ -0,0 +1,14 @@
+class SingletonMeta(type):
+    def __init__(cls, *args, **kwargs):
+        type.__init__(cls, *args, **kwargs)
+        cls._instance = None
+
+    @property
+    def current(cls):
+        if not cls._instance:
+            cls._instance = cls()
+
+        return cls._instance
+
+    def reset(cls, **kwargs):
+        cls._instance = cls(**kwargs)

_appmap/test/data/appmap-all-paths-malformed.yml

@@ -0,0 +1,9 @@
+name: TestApp
+packages:
+- path: abc/xyz
+- path: abc\xyz
+- path: \abc
+- path: xyz/
+- path: 42
+- path: .
+- path: 

_appmap/test/data/appmap-empty-path.yml

@@ -0,0 +1,5 @@
+name: TestApp
+packages:
+  - path: example_class
+  - path:
+

_appmap/test/data/appmap-malformed-path.yml

@@ -0,0 +1,4 @@
+name: TestApp
+packages:
+- path: example_class
+- path: package1/package2/Mod1Class

_appmap/test/test_configuration.py

@@ -26,7 +26,7 @@ def test_can_be_configured():
     """
     assert appmap.enabled()
 
-    c = Config()
+    c = Config.current
     assert c.file_present
     assert c.file_valid
 
@@ -38,7 +38,7 @@ def test_reports_invalid():
     indicates that the config is not valid.
     """
     assert not appmap.enabled()
-    assert not Config().file_valid
+    assert not Config.current.file_valid
 
 
 @pytest.mark.appmap_enabled(config="appmap-broken.yml")
@@ -62,9 +62,9 @@ def test_config_not_found(caplog):
             "APPMAP_CONFIG": "notfound.yml",
         }
     )
-    assert Config().name is None
-    assert not Config().file_present
-    assert not Config().file_valid
+    assert Config.current.name is None
+    assert not Config.current.file_present
+    assert not Config.current.file_valid
 
     assert not appmap.enabled()
     not_found = Path("notfound.yml").resolve()
@@ -80,7 +80,7 @@ def test_config_no_message(caplog):
     """
 
     assert not appmap.enabled()
-    assert Config().name is None
+    assert Config.current.name is None
     assert caplog.text == ""
 
 
@@ -120,6 +120,27 @@ def test_class_prefix_doesnt_match(self):
         f = Filterable(None, "package1_prefix.cls", None)
         assert cf().filter(f) is False
 
+    def test_malformed_path(self, data_dir, caplog):
+        _appmap.initialize(env={"APPMAP_CONFIG": "appmap-malformed-path.yml"}, cwd=data_dir)
+        Config.current._load_config(show_warnings=True)
+        assert (
+            "Malformed path value 'package1/package2/Mod1Class' in configuration file. "
+            "Path entries must be module names not directory paths."
+            in caplog.text
+        )
+
+    def test_all_paths_malformed(self, data_dir):
+        _appmap.initialize(env={"APPMAP_CONFIG": "appmap-all-paths-malformed.yml"}, cwd=data_dir)
+        assert len(Config().packages) == 0
+
+    def test_empty_path(self, data_dir, caplog):
+        _appmap.initialize(env={"APPMAP_CONFIG": "appmap-empty-path.yml"}, cwd=data_dir)
+        Config.current._load_config(show_warnings=True)
+        assert (
+            "Missing path value in configuration file."
+            in caplog.text
+        )
+
 
 class DefaultHelpers:
     def check_default_packages(self, actual_packages):
@@ -129,7 +150,7 @@ def check_default_packages(self, actual_packages):
     def check_default_config(self, expected_name):
         assert appmap.enabled()
 
-        default_config = Config()
+        default_config = Config.current
         assert default_config.name == expected_name
         self.check_default_packages(default_config.packages)
         assert default_config.default["appmap_dir"] == "tmp/appmap"
@@ -160,7 +181,7 @@ def test_skipped_when_overridden(self):
                 "APPMAP_CONFIG": "/tmp/appmap.yml",
             }
         )
-        assert not Config().name
+        assert not Config.current.name
         assert not appmap.enabled()
 
     def test_exclusions(self, data_dir, tmpdir, mocker, monkeypatch):
@@ -186,7 +207,7 @@ def test_created_if_missing_and_enabled(self, git, data_dir, monkeypatch, tmpdir
         # pylint: disable=protected-access
         _appmap.initialize(cwd=repo_root)
 
-        Config()  # write the file as a side-effect
+        Config.current  # write the file as a side-effect
         assert path.is_file()
         with open(path, encoding="utf-8") as cfg:
             actual_config = yaml.safe_load(cfg)
@@ -206,7 +227,7 @@ def test_not_created_if_missing_and_not_enabled(self, git, data_dir, monkeypatch
         # pylint: disable=protected-access
         _appmap.initialize(cwd=repo_root, env={"_APPMAP": "false"})
 
-        c = Config()
+        c = Config.current
         assert not path.is_file()
 
 
@@ -257,7 +278,7 @@ def test_config_in_parent_folder(self, data_dir, tmpdir, monkeypatch):
 
         # pylint: disable=protected-access
         _appmap.initialize(cwd=project_root)
-        assert Config().name == "config-up-name"
+        assert Config.current.name == "config-up-name"
         assert str(Env.current.output_dir).endswith(str(tmpdir / "tmp" / "appmap"))
 
     def test_config_not_found_until_repo_root(self, data_dir, tmpdir, git_directory, monkeypatch):
@@ -272,7 +293,7 @@ def test_config_not_found_until_repo_root(self, data_dir, tmpdir, git_directory,
         # It should stop searching at repo_root.
         # Check that it did not find appmap.yml
         # in config-up folder.
-        assert Config().name != "config-up-name"
+        assert Config.current.name != "config-up-name"
         # It should go on with default config
         assert Env.current.enabled
 
@@ -286,6 +307,6 @@ def test_config_not_found_in_path_hierarchy(self, data_dir, tmpdir, monkeypatch)
             cwd=project_root,
             env={"APPMAP_CONFIG": "notfound.yml"},
         )
-        Config()
+        Config.current
         # No default config since we specified APPMAP_CONFIG
         assert not Env.current.enabled

_appmap/testing_framework.py

@@ -106,7 +106,7 @@ def record(self, klass, method, **kwds):
         metadata = item.metadata
         metadata.update(
             {
-                "app": configuration.Config().name,
+                "app": configuration.Config.current.name,
                 "recorder": {
                     "name": self.name,
                     "type": self.recorder_type,