Change: changes to directory_type, plugin_config validation, plugin i…

…nitializition

- directory_type no longer allows non-existent directories
- Plugin method validate_and_extract_plugin_config split into separate methods with different responsibilities

tests/plugins/test_http_links_in_tags.py

@@ -44,18 +44,14 @@ def test_validate_config(self):
 
         invalid_config_missing_plugin_key = {}
         with self.assertRaises(ConfigurationError) as context:
-            plugin.validate_and_extract_plugin_config(
-                invalid_config_missing_plugin_key
-            )
+            plugin.extract_plugin_config(invalid_config_missing_plugin_key)
         self.assertEqual(
             str(context.exception),
             "Configuration for plugin 'check_http_links_in_tags' is missing.",
         )
-        invalid_config_missing_required_key = {"check_http_links_in_tags": {}}
+        invalid_config_missing_required_key = {}
         with self.assertRaises(ConfigurationError) as context:
-            plugin.validate_and_extract_plugin_config(
-                invalid_config_missing_required_key
-            )
+            plugin.validate_plugin_config(invalid_config_missing_required_key)
         self.assertEqual(
             str(context.exception),
             "Configuration for plugin 'check_http_links_in_tags' "

troubadix/argparser.py

@@ -28,17 +28,18 @@
 
 def directory_type(string: str) -> Path:
     directory_path = Path(string)
-    # Intentionally passes non-existent directories as valid.
-    # DO NOT CHANGE!
-    if directory_path.exists() and not directory_path.is_dir():
+    if not directory_path.is_dir():
         raise ValueError(f"{string} is not a directory.")
     return directory_path
 
 
 def file_type(string: str) -> Path:
+    """if statement is correct and should not be changed
+    checks:
+    - is path an existing file -> file can be used
+    - is a non-existent path -> file can be created at that location later
+    """
     file_path = Path(string)
-    # intentionally passes filepaths that don't exist as valid
-    # DO NOT CHANGE!
     if file_path.exists() and not file_path.is_file():
         raise ValueError(f"{string} is not a file.")
     return file_path

troubadix/plugin.py

@@ -94,25 +94,42 @@ class Plugin(ABC):
 
     def __init__(self, config: dict) -> None:
         if self.require_external_config:
-            self.config = self.validate_and_extract_plugin_config(config)
+            self.config = self.extract_plugin_config(config)
 
-    def validate_and_extract_plugin_config(self, config: dict) -> dict:
+    def extract_plugin_config(self, config: dict) -> dict:
         """
-        Validates and extracts the configuration for a specific plugin
+        extracts the configuration for a specific plugin
         from the entire configuration.
 
-        Not @abstract due to only being necessary
-        if require_external_config is true
-
         Args:
             config (dict): The entire configuration dictionary.
 
         Returns:
             dict: The configuration dictionary for the specific plugin.
 
         Raises:
-            ConfigurationError: If the plugin configuration is not present
-                                or missing required keys.
+            ConfigurationError: If no configuration exists or validation fails.
+        """
+        if self.name not in config:
+            raise ConfigurationError(
+                f"Configuration for plugin '{self.name}' is missing."
+            )
+        plugin_config = config[self.name]
+        self.validate_plugin_config(plugin_config)
+        return plugin_config
+
+    def validate_plugin_config(self, config: dict) -> None:
+        """
+        Validates the configuration for a specific plugin
+
+        Not @abstract due to only being necessary
+        if require_external_config is true
+
+        Args:
+            config (dict): The configuration dictionary for the specific plugin.
+
+        Raises:
+            ConfigurationError: If the plugins required keys are missing.
         """
         raise RuntimeError(
             f"{self.__class__.__name__} has not implemented method"

troubadix/plugins/http_links_in_tags.py

@@ -31,28 +31,16 @@
 
 class CheckHttpLinksInTags(FilePlugin):
     name = "check_http_links_in_tags"
-    mandatory_config_keys = ["exclusions"]
     require_external_config = True
 
-    def validate_and_extract_plugin_config(self, config: dict) -> dict:
-        # check for this plugin in the whole config
-        if self.name not in config:
-            raise ConfigurationError(
-                f"Configuration for plugin '{self.name}' is missing."
-            )
-
-        plugin_config = config[self.name]
-
-        # Check the plugin-specific configuration
-        # for keys required by this plugin
-        if "exclusions" not in plugin_config:
+    def validate_plugin_config(self, config: dict) -> None:
+        """Check the plugin configuration for keys required by this plugin"""
+        if "exclusions" not in config:
             raise ConfigurationError(
                 f"Configuration for plugin '{self.name}' is missing "
                 "required key: 'exclusions'"
             )
 
-        return plugin_config
-
     def run(self) -> Iterator[LinterResult]:
         if self.context.nasl_file.suffix == ".inc":
             return chain()

troubadix/runner.py

@@ -92,32 +92,34 @@ def _check_file(self, file_path: Path) -> FileResults:
             root=self._root, nasl_file=file_path.resolve()
         )
 
-        for plugin_class in self.plugins.file_plugins:
-            if plugin_class.require_external_config:
-                plugin = plugin_class(context, config=self.plugins_config)
-            else:
-                plugin = plugin_class(context)
+        file_plugins = self._initialize_plugins(
+            context, self.plugins.file_plugins
+        )
+        for plugin in file_plugins:
             self._check(plugin, results)
 
         return results
 
+    def _initialize_plugins(self, context, plugin_classes):
+        return [
+            (
+                plugin_class(context, config=self.plugins_config)
+                if plugin_class.require_external_config
+                else plugin_class(context)
+            )
+            for plugin_class in plugin_classes
+        ]
+
     def _run_pooled(self, files: Iterable[Path]):
         """Run all plugins that check single files"""
         self._reporter.set_files_count(len(files))
         with Pool(processes=self._n_jobs, initializer=initializer) as pool:
             try:
                 # run files plugins
                 context = FilesPluginContext(root=self._root, nasl_files=files)
-                files_plugins = [
-                    (
-                        plugin_class(
-                            context, self.plugins_config[plugin_class.name]
-                        )
-                        if plugin_class.require_external_config
-                        else plugin_class(context)
-                    )
-                    for plugin_class in self.plugins.files_plugins
-                ]
+                files_plugins = self._initialize_plugins(
+                    context, self.plugins.files_plugins
+                )
 
                 for results in pool.imap_unordered(
                     self._check_files, files_plugins, chunksize=CHUNKSIZE

troubadix_config.toml

@@ -1,55 +1,56 @@
+# Use Taplo / VS Code Even Better TOML for formatting
+
 title = "Troubadix Ignore File"
 
 [check_http_links_in_tags]
 description = "Strings that should be ignored because they contain a valid URL type in a tag"
 exclusions = [
-            "The payloads try to open a connection to www.google.com",
-            "The script attempts to connect to www.google.com",
-            "to retrieve a web page from www.google.com",
-            "Terms of use at https://www.verisign.com/rpa",
-            "Subject: commonName=www.paypal.com",
-            "example.com",
-            "example.org",
-            "www.exam",
-            "sampling the resolution of a name (www.google.com)",
-            "once with 'www.' and once without",
-            "wget http://www.javaop.com/~ron/tmp/nc",
-            "Ncat: Version 5.30BETA1 (http://nmap.org/ncat)",
-            "as www.windowsupdate.com. (BZ#506016)",
-            "located at http://sambarserver/session/pagecount.",
-            "http://rest.modx.com",
-            "ftp:// ",
-            "ftp://'",
-            "ftp://)",
-            "ftp.c",
-            "ftp.exe",
-            "using special ftp://",
-            "running ftp.",
-            "ftp. The vulnerability",
-            "'http://' protocol",
-            "handle <a href='http://...'> properly",
-            "Switch to git+https://",
-            "wget https://compromised-domain.com/important-file",
-            "the https:// scheme",
-            "https://[email protected]",
-            "distributions on ftp.proftpd.org have all been",
-            "information from www.mutt.org:",
-            "According to www.tcpdump.org:",
-            "According to www.kde.org:",
-            "From the www.info-zip.org site:",
-            " (www.isg.rhul.ac.uk) for discovering this flaw and Adam Langley and",
-            "Sorry about having to reissue this one -- I pulled it from ftp.gnu.org not",
-            # e.g.:
-            # Since gedit supports opening files via 'http://' URLs
-            "'http://'",
-            "'https://'",
-            "http://internal-host$1 is still insecure",
-            "http:// ",
-            "https:// ",
-            "such as 'http://:80'",
-            "<http://localhost/moodle/admin/>",
-            "https://username:password@proxy:8080",
-            "sun.net.www.http.KeepAliveCache",
-            "www.foo.com",
-        ]
-
+    "The payloads try to open a connection to www.google.com",
+    "The script attempts to connect to www.google.com",
+    "to retrieve a web page from www.google.com",
+    "Terms of use at https://www.verisign.com/rpa",
+    "Subject: commonName=www.paypal.com",
+    "example.com",
+    "example.org",
+    "www.exam",
+    "sampling the resolution of a name (www.google.com)",
+    "once with 'www.' and once without",
+    "wget http://www.javaop.com/~ron/tmp/nc",
+    "Ncat: Version 5.30BETA1 (http://nmap.org/ncat)",
+    "as www.windowsupdate.com. (BZ#506016)",
+    "located at http://sambarserver/session/pagecount.",
+    "http://rest.modx.com",
+    "ftp:// ",
+    "ftp://'",
+    "ftp://)",
+    "ftp.c",
+    "ftp.exe",
+    "using special ftp://",
+    "running ftp.",
+    "ftp. The vulnerability",
+    "'http://' protocol",
+    "handle <a href='http://...'> properly",
+    "Switch to git+https://",
+    "wget https://compromised-domain.com/important-file",
+    "the https:// scheme",
+    "https://[email protected]",
+    "distributions on ftp.proftpd.org have all been",
+    "information from www.mutt.org:",
+    "According to www.tcpdump.org:",
+    "According to www.kde.org:",
+    "From the www.info-zip.org site:",
+    " (www.isg.rhul.ac.uk) for discovering this flaw and Adam Langley and",
+    "Sorry about having to reissue this one -- I pulled it from ftp.gnu.org not",
+    # e.g.:
+    # Since gedit supports opening files via 'http://' URLs
+    "'http://'",
+    "'https://'",
+    "http://internal-host$1 is still insecure",
+    "http:// ",
+    "https:// ",
+    "such as 'http://:80'",
+    "<http://localhost/moodle/admin/>",
+    "https://username:password@proxy:8080",
+    "sun.net.www.http.KeepAliveCache",
+    "www.foo.com",
+]