From 742ac22b59898d9b9d3f413e9344f61e8b3f69e5 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Tue, 14 May 2024 18:27:58 +0200 Subject: [PATCH 01/12] Add: Use external toml file for plugin configuration Rewrite http_links_in_tags plugin to use external config for exclusions --- plugins_config.toml | 54 ++++++++++++++++++ poetry.lock | 2 +- pyproject.toml | 1 + tests/plugins/__init__.py | 6 +- tests/plugins/test_http_links_in_tags.py | 72 +++--------------------- tests/test_argparser.py | 16 ++++-- tests/test_runner.py | 16 ++++++ troubadix/argparser.py | 14 ++++- troubadix/plugin.py | 1 + troubadix/plugins/http_links_in_tags.py | 63 ++------------------- troubadix/runner.py | 8 ++- troubadix/troubadix.py | 8 +++ 12 files changed, 128 insertions(+), 133 deletions(-) create mode 100644 plugins_config.toml diff --git a/plugins_config.toml b/plugins_config.toml new file mode 100644 index 00000000..e5b8b2bd --- /dev/null +++ b/plugins_config.toml @@ -0,0 +1,54 @@ +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 properly", + "Switch to git+https://", + "wget https://compromised-domain.com/important-file", + "the https:// scheme", + "https://www.phishingtarget.com@evil.com", + "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'", + "", + "https://username:password@proxy:8080", + "sun.net.www.http.KeepAliveCache", + "www.foo.com", + ] \ No newline at end of file diff --git a/poetry.lock b/poetry.lock index 19cff5ee..4ca37da0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -984,4 +984,4 @@ test = ["flake8 (>=2.4.0)", "isort (>=4.2.2)", "pytest (>=2.2.3)"] [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "b33fd3f05141b11422493b6ae5665de55fffeed3cb4f6e3bffa7048f514f5f96" +content-hash = "556ad95b5c952596849c82b210f6e24a0b02f9d5e1461cbc0c564910e2d7e947" diff --git a/pyproject.toml b/pyproject.toml index 9048f4a3..2c30402f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,7 @@ chardet = ">=4,<6" validators = "0.20.0" gitpython = "^3.1.31" charset-normalizer = "^3.2.0" +tomli = "^2.0.1" [tool.poetry.dev-dependencies] autohooks = ">=21.7.0" diff --git a/tests/plugins/__init__.py b/tests/plugins/__init__.py index 7728df66..97052048 100644 --- a/tests/plugins/__init__.py +++ b/tests/plugins/__init__.py @@ -18,7 +18,7 @@ import tempfile import unittest from pathlib import Path -from typing import Iterable +from typing import Dict, Iterable from unittest.mock import MagicMock from troubadix.plugin import FilePluginContext, FilesPluginContext @@ -56,6 +56,7 @@ def create_file_plugin_context( file_content: str = None, lines: Iterable[str] = None, root: Path = None, + plugin_config: Dict = None, ) -> FilePluginContext: """Create a FilePluginContext mock""" fake_context = MagicMock() @@ -63,6 +64,9 @@ def create_file_plugin_context( fake_context.file_content = file_content fake_context.lines = lines fake_context.root = root + if plugin_config is None: + plugin_config = {} + fake_context.plugin_config = plugin_config return fake_context def create_files_plugin_context( diff --git a/tests/plugins/test_http_links_in_tags.py b/tests/plugins/test_http_links_in_tags.py index 0906d41f..0f9df70d 100644 --- a/tests/plugins/test_http_links_in_tags.py +++ b/tests/plugins/test_http_links_in_tags.py @@ -33,9 +33,16 @@ def test_ok(self): ' script_tag(name:"solution_type", value:"VendorFix");\n' ' script_tag(name:"solution", value:"meh");\n' 'port = get_app_port_from_cpe_prefix("cpe:/o:foo:bar");\n' + ' script_tag(name:"summary", value:"Foo Bar. ' + 'https://www.website.de/demo");\n' ) + fake_plugin_config = { + "exclusions": ["Foo Bar. https://www.website.de/demo"] + } fake_context = self.create_file_plugin_context( - nasl_file=path, file_content=content + nasl_file=path, + file_content=content, + plugin_config=fake_plugin_config, ) plugin = CheckHttpLinksInTags(fake_context) @@ -106,66 +113,3 @@ def test_not_ok2(self): 'value:"https://nvd.nist.gov/vuln/detail/CVE-1234");', results[0].message, ) - - def test_http_link_in_tags_ok(self): - testcases = [ - "01. The payloads try to open a connection to www.google.com", - "02. The script attempts to connect to www.google.com", - "03. to retrieve a web page from www.google.com", - "04. Subject: commonName=www.paypal.com", - "05. Terms of use at https://www.verisign.com/rpa", - "06. example.com", - "07. example.org", - "08. www.exam", - "09. sampling the resolution of a name (www.google.com)", - "10. once with 'www.' and once without", - "11. wget http://www.javaop.com/~ron/tmp/nc", - "12. as www.windowsupdate.com. (BZ#506016)", - "13. located at http://sambarserver/session/pagecount.", - "14. http://rest.modx.com", - "15. ftp:// ", - "16. ftp://'", - "17. ftp://)", - "18. ftp.c", - "19. ftp.exe", - "20. using special ftp://", - "21. running ftp.", - "22. ftp. The vulnerability", - "23. 'http://' protocol", - "24. handle properly", - "25. Switch to git+https://", - "26. wget https://compromised-domain.com/important-file", - "27. the https:// scheme", - "28. https://www.phishingtarget.com@evil.com", - "29. 'http://'", - "30. 'https://'", - "31. distributions on ftp.proftpd.org have all been", - "32. information from www.mutt.org:", - "33. According to www.tcpdump.org:", - "34. According to www.kde.org:", - "35. From the www.info-zip.org site:", - # pylint: disable=line-too-long - "36. (www.isg.rhul.ac.uk) for discovering this flaw and Adam Langley and", - "37. Sorry about having to reissue this one -- I pulled it from ftp.gnu.org not", - "38. http://internal-host$1 is still insecure", - "39. from online sources (ftp://, http:// etc.).", - "40. this and https:// and that.", - "41. such as 'http://:80'", - "42. ", - ] - - for testcase in testcases: - self.assertTrue(CheckHttpLinksInTags.check_to_continue(testcase)) - - def test_http_link_in_tags_not_ok(self): - testcases = [ - "The payloads try to open a connection to www.bing.com", - "examplephishing.org", - "located at http://sambdadancinglessions/session/pagecount.", - "fdp:// ", - "Switch to svn+https://", - "greenbone.net", - ] - - for testcase in testcases: - self.assertFalse(CheckHttpLinksInTags.check_to_continue(testcase)) diff --git a/tests/test_argparser.py b/tests/test_argparser.py index 079e763a..1040194f 100644 --- a/tests/test_argparser.py +++ b/tests/test_argparser.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import tempfile import unittest from multiprocessing import cpu_count from pathlib import Path @@ -134,9 +135,9 @@ def test_parse_min_cpu(self): self.assertEqual(parsed_args.n_jobs, cpu_count() // 2) def test_parse_root(self): - parsed_args = parse_args(self.terminal, ["--root", "foo"]) - - self.assertEqual(parsed_args.root, Path("foo")) + with tempfile.TemporaryDirectory() as tmpdir: + parsed_args = parse_args(self.terminal, ["--root", tmpdir]) + self.assertEqual(parsed_args.root, Path(tmpdir)) def test_parse_fix(self): parsed_args = parse_args(self.terminal, ["--fix"]) @@ -149,6 +150,9 @@ def test_parse_ignore_warnings(self): self.assertTrue(parsed_args.ignore_warnings) def test_parse_log_file_statistic(self): - parsed_args = parse_args(self.terminal, ["--log-file-statistic", "foo"]) - - self.assertEqual(parsed_args.log_file_statistic, Path("foo")) + with tempfile.NamedTemporaryFile() as tmpfile: + print(tmpfile.name) + parsed_args = parse_args( + self.terminal, ["--log-file-statistic", str(tmpfile.name)] + ) + self.assertEqual(parsed_args.log_file_statistic, Path(tmpfile.name)) diff --git a/tests/test_runner.py b/tests/test_runner.py index b13b2cfa..2151e40d 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -51,6 +51,7 @@ def test_runner_with_all_plugins(self): n_jobs=1, reporter=self._reporter, root=self.root, + plugins_config={}, ) plugins = _FILE_PLUGINS + _FILES_PLUGINS @@ -73,6 +74,7 @@ def test_runner_with_excluded_plugins(self): reporter=self._reporter, excluded_plugins=excluded_plugins, root=self.root, + plugins_config={}, ) for plugin in runner.plugins: @@ -88,6 +90,7 @@ def test_runner_with_included_plugins(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) self.assertEqual(len(runner.plugins), 2) @@ -113,6 +116,7 @@ def test_runner_run_ok(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as _: sys_exit = runner.run([nasl_file]) @@ -156,6 +160,7 @@ def test_runner_run_error(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as _: @@ -204,6 +209,7 @@ def test_runner_run_fail_with_verbose_level_2(self): reporter=reporter, included_plugins=[CheckScriptVersionAndLastModificationTags.name], root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -250,6 +256,7 @@ def test_runner_run_ok_with_verbose_level_3(self): reporter=reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -286,6 +293,7 @@ def test_runner_run_ok_with_verbose_level_2(self): reporter=reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -325,6 +333,7 @@ def test_runner_run_ok_with_verbose_level_1(self): n_jobs=1, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -348,6 +357,7 @@ def test_no_plugins(self): reporter=self._reporter, included_plugins=["foo"], root=self.root, + plugins_config={}, ) nasl_file = _here / "plugins" / "test.nasl" @@ -380,6 +390,7 @@ def test_runner_log_file(self): n_jobs=1, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -427,6 +438,7 @@ def test_runner_log_file_fail(self): reporter=reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -464,6 +476,7 @@ def test_runner_run_ok_with_ignore_warnings(self): included_plugins=included_plugins, root=self.root, ignore_warnings=True, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -489,6 +502,7 @@ def test_runner_run_fail_without_ignore_warnings(self): reporter=reporter, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()) as f: @@ -529,6 +543,7 @@ def test_runner_log_file_statistic(self): n_jobs=1, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -575,6 +590,7 @@ def test_runner_fail_log_file_statistic(self): n_jobs=1, included_plugins=included_plugins, root=self.root, + plugins_config={}, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index d4d08e6c..7169a45a 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -28,14 +28,14 @@ def directory_type(string: str) -> Path: directory_path = Path(string) - 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: file_path = Path(string) - if file_path.exists() and not file_path.is_file(): + if not file_path.is_file(): raise ValueError(f"{string} is not a file.") return file_path @@ -230,6 +230,16 @@ def parse_args( help="Don't print the statistic", ) + parser.add_argument( + "--plugins-config-file", + type=file_type, + help=( + "Specify the path to the file that contains additional " + "configuration for the plugins, such as file and " + "other types of exceptions." + ), + ) + if not args: print("No arguments given.", file=sys.stderr) parser.print_help(sys.stdout) diff --git a/troubadix/plugin.py b/troubadix/plugin.py index f04d7350..bb2b3d51 100644 --- a/troubadix/plugin.py +++ b/troubadix/plugin.py @@ -57,6 +57,7 @@ def __init__( self._file_content = None self._lines = None + self.plugin_config = {} @property def file_content(self) -> str: diff --git a/troubadix/plugins/http_links_in_tags.py b/troubadix/plugins/http_links_in_tags.py index eed0a778..1dc0a8cd 100644 --- a/troubadix/plugins/http_links_in_tags.py +++ b/troubadix/plugins/http_links_in_tags.py @@ -17,7 +17,7 @@ import re from itertools import chain -from typing import AnyStr, Iterator +from typing import Iterator from troubadix.helper import SpecialScriptTag, get_common_tag_patterns from troubadix.helper.patterns import get_special_script_tag_pattern @@ -64,7 +64,9 @@ def contains_http_link_in_tag(self) -> Iterator[LinterResult]: if http_link_matches: for http_link_match in http_link_matches: if http_link_match: - if self.check_to_continue(http_link_match.group(0)): + if self.check_to_continue( + http_link_match.group(0), + ): continue yield LinterError( @@ -116,61 +118,8 @@ def contains_nvd_mitre_link_in_xref(self) -> Iterator[LinterResult]: plugin=self.name, ) - @staticmethod - def check_to_continue(http_link_match_group: AnyStr) -> bool: - # When adding new entries to this list, please also add a testcase to - # tests/plugins/test_http_links_in_tags.py -> test_http_link_in_tags_ok - 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", - "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 properly", - "Switch to git+https://", - "wget https://compromised-domain.com/important-file", - "the https:// scheme", - "https://www.phishingtarget.com@evil.com", - "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:", - # pylint: disable=line-too-long - " (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'", - "", - "https://username:password@proxy:8080", - "sun.net.www.http.KeepAliveCache", - "www.foo.com", - ] - + def check_to_continue(self, http_link_match_group: str) -> bool: + exclusions = self.context.plugin_config.get("exclusions", []) return any( exclusion in http_link_match_group for exclusion in exclusions ) diff --git a/troubadix/runner.py b/troubadix/runner.py index e95ada1c..f5e34669 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -19,7 +19,7 @@ import signal from multiprocessing import Pool from pathlib import Path -from typing import Iterable +from typing import Dict, Iterable from troubadix.helper.patterns import ( init_script_tag_patterns, @@ -53,13 +53,15 @@ def __init__( included_plugins: Iterable[str] = None, fix: bool = False, ignore_warnings: bool = False, - ) -> bool: + plugins_config: Dict, + ) -> None: # plugins initialization self.plugins = StandardPlugins(excluded_plugins, included_plugins) self._excluded_plugins = excluded_plugins self._included_plugins = included_plugins + self.plugins_config = plugins_config self._reporter = reporter self._n_jobs = n_jobs self._root = root @@ -91,6 +93,8 @@ def _check_file(self, file_path: Path) -> FileResults: ) for plugin_class in self.plugins.file_plugins: + plugin_config = self.plugins_config.get(plugin_class.name, {}) + context.plugin_config = plugin_config plugin = plugin_class(context) self._check(plugin, results) diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index 1c683601..b767d4b4 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -21,6 +21,7 @@ from pathlib import Path from typing import Iterable, List, Tuple +import tomli from pontos.terminal import Terminal from pontos.terminal.terminal import ConsoleTerminal @@ -163,6 +164,12 @@ def main(args=None): first_file = files[0].resolve() root = get_root(first_file) + if parsed_args.plugins_config_file: + with open(parsed_args.plugins_config_file, "rb") as file: + plugins_config = tomli.load(file) + else: + plugins_config = {} + reporter = Reporter( term=term, fix=parsed_args.fix, @@ -182,6 +189,7 @@ def main(args=None): fix=parsed_args.fix, ignore_warnings=parsed_args.ignore_warnings, root=root, + plugins_config=plugins_config, ) term.info(f"Start linting {len(files)} files ... ") From 516cba3a4ccaae371b7dedaa7a2c9162a1e54cbc Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Mon, 10 Jun 2024 15:12:46 +0200 Subject: [PATCH 02/12] Change: Comment that parser types are intended to be the way they are --- plugins_config.toml | 3 ++- tests/plugins/__init__.py | 4 ++-- troubadix/argparser.py | 8 ++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins_config.toml b/plugins_config.toml index e5b8b2bd..d46719ff 100644 --- a/plugins_config.toml +++ b/plugins_config.toml @@ -51,4 +51,5 @@ exclusions = [ "https://username:password@proxy:8080", "sun.net.www.http.KeepAliveCache", "www.foo.com", - ] \ No newline at end of file + ] + diff --git a/tests/plugins/__init__.py b/tests/plugins/__init__.py index 97052048..177f1686 100644 --- a/tests/plugins/__init__.py +++ b/tests/plugins/__init__.py @@ -18,7 +18,7 @@ import tempfile import unittest from pathlib import Path -from typing import Dict, Iterable +from typing import Iterable from unittest.mock import MagicMock from troubadix.plugin import FilePluginContext, FilesPluginContext @@ -56,7 +56,7 @@ def create_file_plugin_context( file_content: str = None, lines: Iterable[str] = None, root: Path = None, - plugin_config: Dict = None, + plugin_config: dict = None, ) -> FilePluginContext: """Create a FilePluginContext mock""" fake_context = MagicMock() diff --git a/troubadix/argparser.py b/troubadix/argparser.py index 7169a45a..8dd9ffbc 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -28,14 +28,18 @@ def directory_type(string: str) -> Path: directory_path = Path(string) - if not directory_path.is_dir(): + # Intentionally passes non-existent directories as valid. + # DO NOT CHANGE! + if directory_path.exists() and not directory_path.is_dir(): raise ValueError(f"{string} is not a directory.") return directory_path def file_type(string: str) -> Path: file_path = Path(string) - if not file_path.is_file(): + # 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 From d85723a6745e609d8fcc11dfa8af997f2fb9a4b3 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Thu, 13 Jun 2024 12:25:55 +0200 Subject: [PATCH 03/12] Change: Make config file required, Separate plugin config from file context --- tests/plugins/test_http_links_in_tags.py | 49 +++++++++++++++-- tests/test_argparser.py | 70 ++++++++++++++++++++++-- troubadix/argparser.py | 7 +++ troubadix/plugin.py | 46 +++++++++++++++- troubadix/plugins/http_links_in_tags.py | 35 +++++++++++- troubadix/runner.py | 18 ++++-- troubadix/troubadix.py | 10 +++- 7 files changed, 210 insertions(+), 25 deletions(-) diff --git a/tests/plugins/test_http_links_in_tags.py b/tests/plugins/test_http_links_in_tags.py index 0f9df70d..a47292c4 100644 --- a/tests/plugins/test_http_links_in_tags.py +++ b/tests/plugins/test_http_links_in_tags.py @@ -18,13 +18,50 @@ from pathlib import Path -from troubadix.plugin import LinterError +from troubadix.plugin import ConfigurationError, LinterError from troubadix.plugins.http_links_in_tags import CheckHttpLinksInTags from . import PluginTestCase +BASE_CONFIG = {CheckHttpLinksInTags.name: {"exclusions": []}} + class CheckHttpLinksInTagsTestCase(PluginTestCase): + + def test_validate_config(self): + fake_context = self.create_file_plugin_context() + + valid_plugin_config = { + "exclusions": ["Foo Bar. https://www.website.de/demo"] + } + valid_config = {"check_http_links_in_tags": valid_plugin_config} + + # validate_and_extract_config is called by the init method + # when key config is given. + plugin = CheckHttpLinksInTags(fake_context, config=valid_config) + + self.assertEqual(plugin.config, valid_plugin_config) + + invalid_config_missing_plugin_key = {} + with self.assertRaises(ConfigurationError) as context: + plugin.validate_and_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": {}} + with self.assertRaises(ConfigurationError) as context: + plugin.validate_and_extract_plugin_config( + invalid_config_missing_required_key + ) + self.assertEqual( + str(context.exception), + "Configuration for plugin 'check_http_links_in_tags' " + "is missing required key: 'exclusions'", + ) + def test_ok(self): path = Path("some/file.nasl") content = ( @@ -42,9 +79,9 @@ def test_ok(self): fake_context = self.create_file_plugin_context( nasl_file=path, file_content=content, - plugin_config=fake_plugin_config, ) - plugin = CheckHttpLinksInTags(fake_context) + plugin = CheckHttpLinksInTags(fake_context, config=BASE_CONFIG) + plugin.config = fake_plugin_config results = list(plugin.run()) @@ -53,7 +90,7 @@ def test_ok(self): def test_exclude_inc_file(self): path = Path("some/file.inc") fake_context = self.create_file_plugin_context(nasl_file=path) - plugin = CheckHttpLinksInTags(fake_context) + plugin = CheckHttpLinksInTags(fake_context, config=BASE_CONFIG) results = list(plugin.run()) @@ -71,7 +108,7 @@ def test_not_ok(self): fake_context = self.create_file_plugin_context( nasl_file=path, file_content=content ) - plugin = CheckHttpLinksInTags(fake_context) + plugin = CheckHttpLinksInTags(fake_context, config=BASE_CONFIG) results = list(plugin.run()) @@ -99,7 +136,7 @@ def test_not_ok2(self): fake_context = self.create_file_plugin_context( nasl_file=path, file_content=content ) - plugin = CheckHttpLinksInTags(fake_context) + plugin = CheckHttpLinksInTags(fake_context, config=BASE_CONFIG) results = list(plugin.run()) diff --git a/tests/test_argparser.py b/tests/test_argparser.py index 1040194f..10cbbe57 100644 --- a/tests/test_argparser.py +++ b/tests/test_argparser.py @@ -37,6 +37,8 @@ def test_parse_files(self): "--files", "tests/plugins/test.nasl", "tests/plugins/fail2.nasl", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) @@ -52,6 +54,8 @@ def test_parse_include_tests(self): [ "--include-tests", "CheckBadwords", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) self.assertEqual(parsed_args.included_plugins, ["CheckBadwords"]) @@ -63,6 +67,8 @@ def test_parse_exclude_tests(self): [ "--exclude-tests", "CheckBadwords", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) self.assertEqual(parsed_args.excluded_plugins, ["CheckBadwords"]) @@ -70,7 +76,14 @@ def test_parse_exclude_tests(self): def test_parse_include_patterns(self): parsed_args = parse_args( - self.terminal, ["-f", "--include-patterns", "troubadix/*"] + self.terminal, + [ + "-f", + "--include-patterns", + "troubadix/*", + "--plugins-config-file", + "some/fake/path/config.toml", + ], ) self.assertTrue(parsed_args.full) @@ -89,12 +102,21 @@ def test_parse_files_non_recursive_fail(self): "tests/plugins/test.nasl", "tests/plugins/fail2.nasl", "--non-recursive", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) def test_parse_exclude_patterns(self): parsed_args = parse_args( - self.terminal, ["-f", "--exclude-patterns", "troubadix/*"] + self.terminal, + [ + "-f", + "--exclude-patterns", + "troubadix/*", + "--plugins-config-file", + "some/fake/path/config.toml", + ], ) self.assertTrue(parsed_args.full) @@ -109,6 +131,8 @@ def test_parse_max_cpu(self): "troubadix/*", "-j", "1337", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) @@ -126,6 +150,8 @@ def test_parse_min_cpu(self): "troubadix/*", "-j", "-1337", + "--plugins-config-file", + "some/fake/path/config.toml", ], ) @@ -136,16 +162,38 @@ def test_parse_min_cpu(self): def test_parse_root(self): with tempfile.TemporaryDirectory() as tmpdir: - parsed_args = parse_args(self.terminal, ["--root", tmpdir]) + parsed_args = parse_args( + self.terminal, + [ + "--root", + tmpdir, + "--plugins-config-file", + "some/fake/path/config.toml", + ], + ) self.assertEqual(parsed_args.root, Path(tmpdir)) def test_parse_fix(self): - parsed_args = parse_args(self.terminal, ["--fix"]) + parsed_args = parse_args( + self.terminal, + [ + "--fix", + "--plugins-config-file", + "some/fake/path/config.toml", + ], + ) self.assertTrue(parsed_args.fix) def test_parse_ignore_warnings(self): - parsed_args = parse_args(self.terminal, ["--ignore-warnings"]) + parsed_args = parse_args( + self.terminal, + [ + "--ignore-warnings", + "--plugins-config-file", + "some/fake/path/config.toml", + ], + ) self.assertTrue(parsed_args.ignore_warnings) @@ -153,6 +201,16 @@ def test_parse_log_file_statistic(self): with tempfile.NamedTemporaryFile() as tmpfile: print(tmpfile.name) parsed_args = parse_args( - self.terminal, ["--log-file-statistic", str(tmpfile.name)] + self.terminal, + [ + "--log-file-statistic", + str(tmpfile.name), + "--plugins-config-file", + "some/fake/path/config.toml", + ], ) self.assertEqual(parsed_args.log_file_statistic, Path(tmpfile.name)) + + def test_parse_config_missing(self): + with self.assertRaises(SystemExit): + parse_args(self.terminal, []) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index 8dd9ffbc..de6b2859 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -273,4 +273,11 @@ def parse_args( ) sys.exit(1) + if not parsed_args.plugins_config_file: + terminal.warning( + "A plugin_config file needs to be specified with " + " '--plugins_config_file'" + ) + sys.exit(1) + return parsed_args diff --git a/troubadix/plugin.py b/troubadix/plugin.py index bb2b3d51..ad3fbcc3 100644 --- a/troubadix/plugin.py +++ b/troubadix/plugin.py @@ -57,7 +57,6 @@ def __init__( self._file_content = None self._lines = None - self.plugin_config = {} @property def file_content(self) -> str: @@ -80,12 +79,48 @@ def __init__(self, *, root: Path, nasl_files: Iterable[Path]) -> None: self.nasl_files = nasl_files +class ConfigurationError(Exception): + """Custom exception for plugin_configurion errors.""" + + class Plugin(ABC): """A linter plugin""" name: str = None description: str = None + # Value to indicate that a plugin depends on an external configuration + require_external_config = False + + def __init__(self, config: dict) -> None: + if self.require_external_config: + self.config = self.validate_and_extract_plugin_config(config) + + def validate_and_extract_plugin_config(self, config: dict) -> dict: + """ + Validates and 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. + """ + raise RuntimeError( + f"{self.__class__.__name__} has not implemented method" + " 'validate_and_extract_plugin_config'." + " This method should be overridden in subclasses," + " if they require external config" + ) + @abstractmethod def run(self) -> Iterator[LinterResult]: pass @@ -97,14 +132,19 @@ def fix(self) -> Iterator[LinterResult]: class FilesPlugin(Plugin): """A plugin that does checks over all files""" - def __init__(self, context: FilesPluginContext) -> None: + def __init__(self, context: FilesPluginContext, **kwargs) -> None: + if "config" in kwargs: + super().__init__(kwargs["config"]) self.context = context class FilePlugin(Plugin): """A plugin that does checks on single files""" - def __init__(self, context: FilePluginContext) -> None: + def __init__(self, context: FilePluginContext, **kwargs) -> None: + if "config" in kwargs: + super().__init__(kwargs["config"]) + self.context = context diff --git a/troubadix/plugins/http_links_in_tags.py b/troubadix/plugins/http_links_in_tags.py index 1dc0a8cd..9c704d1f 100644 --- a/troubadix/plugins/http_links_in_tags.py +++ b/troubadix/plugins/http_links_in_tags.py @@ -21,11 +21,37 @@ from troubadix.helper import SpecialScriptTag, get_common_tag_patterns from troubadix.helper.patterns import get_special_script_tag_pattern -from troubadix.plugin import FilePlugin, LinterError, LinterResult +from troubadix.plugin import ( + ConfigurationError, + FilePlugin, + LinterError, + LinterResult, +) 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: + 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": @@ -49,6 +75,11 @@ def contains_http_link_in_tag(self) -> Iterator[LinterResult]: nasl_file: The VT that is going to be checked file_content: The content of the file that is going to be checked + config: The plugin configuration provided + by the plugin_configuration.toml file. + config must include keys: + exclusions: A list of Strings that should be ignored + due to containing a valid use of a url in a tag """ file_content = self.context.file_content @@ -119,7 +150,7 @@ def contains_nvd_mitre_link_in_xref(self) -> Iterator[LinterResult]: ) def check_to_continue(self, http_link_match_group: str) -> bool: - exclusions = self.context.plugin_config.get("exclusions", []) + exclusions = self.config["exclusions"] return any( exclusion in http_link_match_group for exclusion in exclusions ) diff --git a/troubadix/runner.py b/troubadix/runner.py index f5e34669..f9ae6644 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -93,10 +93,10 @@ def _check_file(self, file_path: Path) -> FileResults: ) for plugin_class in self.plugins.file_plugins: - plugin_config = self.plugins_config.get(plugin_class.name, {}) - context.plugin_config = plugin_config - plugin = plugin_class(context) - + if plugin_class.require_external_config: + plugin = plugin_class(context, config=self.plugins_config) + else: + plugin = plugin_class(context) self._check(plugin, results) return results @@ -109,7 +109,13 @@ def _run_pooled(self, files: Iterable[Path]): # run files plugins context = FilesPluginContext(root=self._root, nasl_files=files) files_plugins = [ - plugin_class(context) + ( + 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 ] @@ -139,6 +145,8 @@ def run(self, files: Iterable[Path]) -> bool: if not len(self.plugins): raise TroubadixException("No Plugin found.") + # self._check_plugins_config_keys() + # print plugins that will be executed self._reporter.report_plugin_overview( plugins=self.plugins, diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index b767d4b4..2e56c817 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -164,11 +164,15 @@ def main(args=None): first_file = files[0].resolve() root = get_root(first_file) - if parsed_args.plugins_config_file: + # Get the plugins configurations from the external toml file + try: with open(parsed_args.plugins_config_file, "rb") as file: plugins_config = tomli.load(file) - else: - plugins_config = {} + except FileNotFoundError: + term.warning( + f"Config file '{parsed_args.plugins_config_file}' does not exist" + ) + sys.exit(1) reporter = Reporter( term=term, From 63910998db0e96807fc9a1d97f2340648b95ee75 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Thu, 13 Jun 2024 16:13:55 +0200 Subject: [PATCH 04/12] Change: Use default value for plugins-config-file parser argument --- tests/plugins/__init__.py | 4 -- tests/test_argparser.py | 70 ++------------------ troubadix/argparser.py | 8 +-- troubadix/runner.py | 4 +- plugins_config.toml => troubadix_config.toml | 0 5 files changed, 9 insertions(+), 77 deletions(-) rename plugins_config.toml => troubadix_config.toml (100%) diff --git a/tests/plugins/__init__.py b/tests/plugins/__init__.py index 177f1686..7728df66 100644 --- a/tests/plugins/__init__.py +++ b/tests/plugins/__init__.py @@ -56,7 +56,6 @@ def create_file_plugin_context( file_content: str = None, lines: Iterable[str] = None, root: Path = None, - plugin_config: dict = None, ) -> FilePluginContext: """Create a FilePluginContext mock""" fake_context = MagicMock() @@ -64,9 +63,6 @@ def create_file_plugin_context( fake_context.file_content = file_content fake_context.lines = lines fake_context.root = root - if plugin_config is None: - plugin_config = {} - fake_context.plugin_config = plugin_config return fake_context def create_files_plugin_context( diff --git a/tests/test_argparser.py b/tests/test_argparser.py index 10cbbe57..1040194f 100644 --- a/tests/test_argparser.py +++ b/tests/test_argparser.py @@ -37,8 +37,6 @@ def test_parse_files(self): "--files", "tests/plugins/test.nasl", "tests/plugins/fail2.nasl", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) @@ -54,8 +52,6 @@ def test_parse_include_tests(self): [ "--include-tests", "CheckBadwords", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) self.assertEqual(parsed_args.included_plugins, ["CheckBadwords"]) @@ -67,8 +63,6 @@ def test_parse_exclude_tests(self): [ "--exclude-tests", "CheckBadwords", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) self.assertEqual(parsed_args.excluded_plugins, ["CheckBadwords"]) @@ -76,14 +70,7 @@ def test_parse_exclude_tests(self): def test_parse_include_patterns(self): parsed_args = parse_args( - self.terminal, - [ - "-f", - "--include-patterns", - "troubadix/*", - "--plugins-config-file", - "some/fake/path/config.toml", - ], + self.terminal, ["-f", "--include-patterns", "troubadix/*"] ) self.assertTrue(parsed_args.full) @@ -102,21 +89,12 @@ def test_parse_files_non_recursive_fail(self): "tests/plugins/test.nasl", "tests/plugins/fail2.nasl", "--non-recursive", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) def test_parse_exclude_patterns(self): parsed_args = parse_args( - self.terminal, - [ - "-f", - "--exclude-patterns", - "troubadix/*", - "--plugins-config-file", - "some/fake/path/config.toml", - ], + self.terminal, ["-f", "--exclude-patterns", "troubadix/*"] ) self.assertTrue(parsed_args.full) @@ -131,8 +109,6 @@ def test_parse_max_cpu(self): "troubadix/*", "-j", "1337", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) @@ -150,8 +126,6 @@ def test_parse_min_cpu(self): "troubadix/*", "-j", "-1337", - "--plugins-config-file", - "some/fake/path/config.toml", ], ) @@ -162,38 +136,16 @@ def test_parse_min_cpu(self): def test_parse_root(self): with tempfile.TemporaryDirectory() as tmpdir: - parsed_args = parse_args( - self.terminal, - [ - "--root", - tmpdir, - "--plugins-config-file", - "some/fake/path/config.toml", - ], - ) + parsed_args = parse_args(self.terminal, ["--root", tmpdir]) self.assertEqual(parsed_args.root, Path(tmpdir)) def test_parse_fix(self): - parsed_args = parse_args( - self.terminal, - [ - "--fix", - "--plugins-config-file", - "some/fake/path/config.toml", - ], - ) + parsed_args = parse_args(self.terminal, ["--fix"]) self.assertTrue(parsed_args.fix) def test_parse_ignore_warnings(self): - parsed_args = parse_args( - self.terminal, - [ - "--ignore-warnings", - "--plugins-config-file", - "some/fake/path/config.toml", - ], - ) + parsed_args = parse_args(self.terminal, ["--ignore-warnings"]) self.assertTrue(parsed_args.ignore_warnings) @@ -201,16 +153,6 @@ def test_parse_log_file_statistic(self): with tempfile.NamedTemporaryFile() as tmpfile: print(tmpfile.name) parsed_args = parse_args( - self.terminal, - [ - "--log-file-statistic", - str(tmpfile.name), - "--plugins-config-file", - "some/fake/path/config.toml", - ], + self.terminal, ["--log-file-statistic", str(tmpfile.name)] ) self.assertEqual(parsed_args.log_file_statistic, Path(tmpfile.name)) - - def test_parse_config_missing(self): - with self.assertRaises(SystemExit): - parse_args(self.terminal, []) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index de6b2859..a6e8578c 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -237,6 +237,7 @@ def parse_args( parser.add_argument( "--plugins-config-file", type=file_type, + default="troubadix_config.toml", help=( "Specify the path to the file that contains additional " "configuration for the plugins, such as file and " @@ -273,11 +274,4 @@ def parse_args( ) sys.exit(1) - if not parsed_args.plugins_config_file: - terminal.warning( - "A plugin_config file needs to be specified with " - " '--plugins_config_file'" - ) - sys.exit(1) - return parsed_args diff --git a/troubadix/runner.py b/troubadix/runner.py index f9ae6644..e8ec0545 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -19,7 +19,7 @@ import signal from multiprocessing import Pool from pathlib import Path -from typing import Dict, Iterable +from typing import Iterable from troubadix.helper.patterns import ( init_script_tag_patterns, @@ -53,7 +53,7 @@ def __init__( included_plugins: Iterable[str] = None, fix: bool = False, ignore_warnings: bool = False, - plugins_config: Dict, + plugins_config: dict, ) -> None: # plugins initialization self.plugins = StandardPlugins(excluded_plugins, included_plugins) diff --git a/plugins_config.toml b/troubadix_config.toml similarity index 100% rename from plugins_config.toml rename to troubadix_config.toml From fbb4e8e444c54372e6fbdcae950f58a8202fe22b Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Mon, 17 Jun 2024 11:07:58 +0200 Subject: [PATCH 05/12] Change: fix typo, remove comment --- troubadix/plugin.py | 2 +- troubadix/runner.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/troubadix/plugin.py b/troubadix/plugin.py index ad3fbcc3..1f225cce 100644 --- a/troubadix/plugin.py +++ b/troubadix/plugin.py @@ -80,7 +80,7 @@ def __init__(self, *, root: Path, nasl_files: Iterable[Path]) -> None: class ConfigurationError(Exception): - """Custom exception for plugin_configurion errors.""" + """Custom exception for plugin_configuration errors.""" class Plugin(ABC): diff --git a/troubadix/runner.py b/troubadix/runner.py index e8ec0545..ceee463d 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -145,8 +145,6 @@ def run(self, files: Iterable[Path]) -> bool: if not len(self.plugins): raise TroubadixException("No Plugin found.") - # self._check_plugins_config_keys() - # print plugins that will be executed self._reporter.report_plugin_overview( plugins=self.plugins, From b65734a9a6a3e39012931919a8099deb80309159 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Tue, 18 Jun 2024 17:26:35 +0200 Subject: [PATCH 06/12] Change: changes to directory_type, plugin_config validation, plugin initializition - 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 | 14 ++- tests/test_argparser.py | 1 - tests/test_runner.py | 34 ++++---- troubadix/argparser.py | 11 +-- troubadix/plugin.py | 33 ++++++-- troubadix/plugins/http_links_in_tags.py | 18 +--- troubadix/runner.py | 32 +++---- troubadix_config.toml | 103 ++++++++++++----------- 8 files changed, 127 insertions(+), 119 deletions(-) diff --git a/tests/plugins/test_http_links_in_tags.py b/tests/plugins/test_http_links_in_tags.py index a47292c4..b3583999 100644 --- a/tests/plugins/test_http_links_in_tags.py +++ b/tests/plugins/test_http_links_in_tags.py @@ -36,25 +36,23 @@ def test_validate_config(self): } valid_config = {"check_http_links_in_tags": valid_plugin_config} - # validate_and_extract_config is called by the init method - # when key config is given. + # config extraction and validation is done when the init method + # is called with the optional key config. plugin = CheckHttpLinksInTags(fake_context, config=valid_config) self.assertEqual(plugin.config, valid_plugin_config) 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_plugin_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_plugin_config_missing_required_key ) self.assertEqual( str(context.exception), diff --git a/tests/test_argparser.py b/tests/test_argparser.py index 1040194f..c2de5d83 100644 --- a/tests/test_argparser.py +++ b/tests/test_argparser.py @@ -151,7 +151,6 @@ def test_parse_ignore_warnings(self): def test_parse_log_file_statistic(self): with tempfile.NamedTemporaryFile() as tmpfile: - print(tmpfile.name) parsed_args = parse_args( self.terminal, ["--log-file-statistic", str(tmpfile.name)] ) diff --git a/tests/test_runner.py b/tests/test_runner.py index 2151e40d..2779cf67 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -39,6 +39,8 @@ _here = Path(__file__).parent +DEFAULT_CONFIG = {} + class TestRunner(unittest.TestCase): def setUp(self): @@ -51,7 +53,7 @@ def test_runner_with_all_plugins(self): n_jobs=1, reporter=self._reporter, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) plugins = _FILE_PLUGINS + _FILES_PLUGINS @@ -74,7 +76,7 @@ def test_runner_with_excluded_plugins(self): reporter=self._reporter, excluded_plugins=excluded_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) for plugin in runner.plugins: @@ -90,7 +92,7 @@ def test_runner_with_included_plugins(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) self.assertEqual(len(runner.plugins), 2) @@ -116,7 +118,7 @@ def test_runner_run_ok(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as _: sys_exit = runner.run([nasl_file]) @@ -160,7 +162,7 @@ def test_runner_run_error(self): reporter=self._reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as _: @@ -209,7 +211,7 @@ def test_runner_run_fail_with_verbose_level_2(self): reporter=reporter, included_plugins=[CheckScriptVersionAndLastModificationTags.name], root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -256,7 +258,7 @@ def test_runner_run_ok_with_verbose_level_3(self): reporter=reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -293,7 +295,7 @@ def test_runner_run_ok_with_verbose_level_2(self): reporter=reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -333,7 +335,7 @@ def test_runner_run_ok_with_verbose_level_1(self): n_jobs=1, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -357,7 +359,7 @@ def test_no_plugins(self): reporter=self._reporter, included_plugins=["foo"], root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) nasl_file = _here / "plugins" / "test.nasl" @@ -390,7 +392,7 @@ def test_runner_log_file(self): n_jobs=1, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -438,7 +440,7 @@ def test_runner_log_file_fail(self): reporter=reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -476,7 +478,7 @@ def test_runner_run_ok_with_ignore_warnings(self): included_plugins=included_plugins, root=self.root, ignore_warnings=True, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -502,7 +504,7 @@ def test_runner_run_fail_without_ignore_warnings(self): reporter=reporter, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()) as f: @@ -543,7 +545,7 @@ def test_runner_log_file_statistic(self): n_jobs=1, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) @@ -590,7 +592,7 @@ def test_runner_fail_log_file_statistic(self): n_jobs=1, included_plugins=included_plugins, root=self.root, - plugins_config={}, + plugins_config=DEFAULT_CONFIG, ) with redirect_stdout(io.StringIO()): runner.run([nasl_file]) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index a6e8578c..a0bb048b 100644 --- a/troubadix/argparser.py +++ b/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 diff --git a/troubadix/plugin.py b/troubadix/plugin.py index 1f225cce..9e20984c 100644 --- a/troubadix/plugin.py +++ b/troubadix/plugin.py @@ -94,16 +94,13 @@ 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. @@ -111,8 +108,28 @@ def validate_and_extract_plugin_config(self, config: dict) -> dict: 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" diff --git a/troubadix/plugins/http_links_in_tags.py b/troubadix/plugins/http_links_in_tags.py index 9c704d1f..5734bc05 100644 --- a/troubadix/plugins/http_links_in_tags.py +++ b/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() diff --git a/troubadix/runner.py b/troubadix/runner.py index ceee463d..126fb8b4 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -92,15 +92,24 @@ 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)) @@ -108,16 +117,9 @@ def _run_pooled(self, files: Iterable[Path]): 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 diff --git a/troubadix_config.toml b/troubadix_config.toml index d46719ff..1215186c 100644 --- a/troubadix_config.toml +++ b/troubadix_config.toml @@ -1,55 +1,56 @@ -title = "Troubadix Ignore File" +# Use Taplo / VS Code Even Better TOML for formatting + +title = "Troubadix Plugin Config 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 properly", - "Switch to git+https://", - "wget https://compromised-domain.com/important-file", - "the https:// scheme", - "https://www.phishingtarget.com@evil.com", - "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'", - "", - "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 properly", + "Switch to git+https://", + "wget https://compromised-domain.com/important-file", + "the https:// scheme", + "https://www.phishingtarget.com@evil.com", + "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'", + "", + "https://username:password@proxy:8080", + "sun.net.www.http.KeepAliveCache", + "www.foo.com", +] From a991dab221a8259777637dce9e9009a4f7128a7c Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Tue, 2 Jul 2024 12:57:04 +0200 Subject: [PATCH 07/12] Change: remove unneeded exlusion --- troubadix_config.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/troubadix_config.toml b/troubadix_config.toml index 1215186c..98aa20c6 100644 --- a/troubadix_config.toml +++ b/troubadix_config.toml @@ -16,7 +16,6 @@ exclusions = [ "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", From 2d75489ddce722d3b29342f3241b97c1b1c01fff Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Wed, 3 Jul 2024 17:00:48 +0200 Subject: [PATCH 08/12] Change: Use conditional import of tomllib --- poetry.lock | 2 +- pyproject.toml | 11 ++++------- troubadix/troubadix.py | 8 ++++++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/poetry.lock b/poetry.lock index 4ca37da0..029f3ed4 100644 --- a/poetry.lock +++ b/poetry.lock @@ -984,4 +984,4 @@ test = ["flake8 (>=2.4.0)", "isort (>=4.2.2)", "pytest (>=2.2.3)"] [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "556ad95b5c952596849c82b210f6e24a0b02f9d5e1461cbc0c564910e2d7e947" +content-hash = "ac5bd99d673cdfcd48512b6f97ab6cf729a39ba89760c33dc7851749f26a7965" diff --git a/pyproject.toml b/pyproject.toml index 2c30402f..ef7179d9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,9 +9,9 @@ repository = "https://github.com/greenbone/troubadix" homepage = "https://github.com/greenbone/troubadix" # Full list: https://pypi.org/pypi?%3Aaction=list_classifiers -classifiers=[ +classifiers = [ "Development Status :: 4 - Beta", - "License :: OSI Approved :: GNU General Public License v3 or later (GPLv3+)", # pylint: disable=line-too-long + "License :: OSI Approved :: GNU General Public License v3 or later (GPLv3+)", # pylint: disable=line-too-long "Environment :: Console", "Intended Audience :: Developers", "Programming Language :: Python :: 3.9", @@ -21,10 +21,7 @@ classifiers=[ "Topic :: Software Development :: Libraries :: Python Modules", ] -packages = [ - { include = "troubadix"}, - { include = "tests", format = "sdist" }, -] +packages = [{ include = "troubadix" }, { include = "tests", format = "sdist" }] [tool.poetry.dependencies] python = "^3.9" @@ -35,7 +32,7 @@ chardet = ">=4,<6" validators = "0.20.0" gitpython = "^3.1.31" charset-normalizer = "^3.2.0" -tomli = "^2.0.1" +tomli = { version = "^2.0.1", python = "<3.11" } [tool.poetry.dev-dependencies] autohooks = ">=21.7.0" diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index 2e56c817..de0abed0 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -21,7 +21,6 @@ from pathlib import Path from typing import Iterable, List, Tuple -import tomli from pontos.terminal import Terminal from pontos.terminal.terminal import ConsoleTerminal @@ -31,6 +30,11 @@ from troubadix.reporter import Reporter from troubadix.runner import Runner +try: + import tomllib +except ImportError: + import tomli as tomllib + def generate_file_list( dirs: Iterable[Path], @@ -167,7 +171,7 @@ def main(args=None): # Get the plugins configurations from the external toml file try: with open(parsed_args.plugins_config_file, "rb") as file: - plugins_config = tomli.load(file) + plugins_config = tomllib.load(file) except FileNotFoundError: term.warning( f"Config file '{parsed_args.plugins_config_file}' does not exist" From de8d0b7e8b07473600cdfdf1ef6ad8cdf102b88e Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Mon, 8 Jul 2024 09:20:50 +0200 Subject: [PATCH 09/12] Change: move config file default location and change flag --- troubadix_config.toml => troubadix.toml | 7 +++++-- troubadix/argparser.py | 8 ++++---- troubadix/troubadix.py | 6 ++---- 3 files changed, 11 insertions(+), 10 deletions(-) rename troubadix_config.toml => troubadix.toml (91%) diff --git a/troubadix_config.toml b/troubadix.toml similarity index 91% rename from troubadix_config.toml rename to troubadix.toml index 98aa20c6..127adf94 100644 --- a/troubadix_config.toml +++ b/troubadix.toml @@ -1,9 +1,12 @@ # Use Taplo / VS Code Even Better TOML for formatting -title = "Troubadix Plugin Config File" +# File Structure: +# [plugin_name] +# config_a = value_a +# config_b = value_b [check_http_links_in_tags] -description = "Strings that should be ignored because they contain a valid URL type in a tag" +# 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", diff --git a/troubadix/argparser.py b/troubadix/argparser.py index a0bb048b..da8a9343 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -236,13 +236,13 @@ def parse_args( ) parser.add_argument( - "--plugins-config-file", + "-c", + "--config", type=file_type, - default="troubadix_config.toml", + default="troubadix.toml", help=( "Specify the path to the file that contains additional " - "configuration for the plugins, such as file and " - "other types of exceptions." + "configuration for the plugins" ), ) diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index de0abed0..46762002 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -170,12 +170,10 @@ def main(args=None): # Get the plugins configurations from the external toml file try: - with open(parsed_args.plugins_config_file, "rb") as file: + with open(parsed_args.config, "rb") as file: plugins_config = tomllib.load(file) except FileNotFoundError: - term.warning( - f"Config file '{parsed_args.plugins_config_file}' does not exist" - ) + term.warning(f"Config file '{parsed_args.config}' does not exist") sys.exit(1) reporter = Reporter( From 2c291d15c73319393750e544110e8aef29fe5590 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Mon, 8 Jul 2024 11:16:34 +0200 Subject: [PATCH 10/12] Change: use tmp_dirs in test_deprecate_vts due to file_type change --- .../standalone_plugins/test_deprecate_vts.py | 112 +++++++++--------- 1 file changed, 57 insertions(+), 55 deletions(-) diff --git a/tests/standalone_plugins/test_deprecate_vts.py b/tests/standalone_plugins/test_deprecate_vts.py index 1e5715c7..66709f55 100644 --- a/tests/standalone_plugins/test_deprecate_vts.py +++ b/tests/standalone_plugins/test_deprecate_vts.py @@ -4,88 +4,90 @@ # pylint: disable=protected-access import unittest from pathlib import Path -from tests.plugins import TemporaryDirectory +from tests.plugins import TemporaryDirectory from troubadix.standalone_plugins.deprecate_vts import ( - deprecate, - parse_args, DeprecatedFile, - _get_summary, _finalize_content, - update_summary, - get_files_from_path, + _get_summary, + deprecate, filter_files, + get_files_from_path, + parse_args, + update_summary, ) class ParseArgsTestCase(unittest.TestCase): def test_parse_args(self): testfile = "testfile.nasl" - output_path = "attic/" - reason = "NOTUS" - - args = parse_args( - [ - "--files", - testfile, - "--output-path", - output_path, - "--deprecation-reason", - reason, - ] - ) - self.assertEqual(args.files, [Path(testfile)]) - self.assertEqual(args.output_path, Path(output_path)) - self.assertEqual(args.deprecation_reason, reason) - - def test_mandatory_arg_group_both(self): - testfile = "testfile.nasl" - output_path = "attic/" - input_path = "nasl/common" reason = "NOTUS" - with self.assertRaises(SystemExit): - parse_args( + with TemporaryDirectory() as out_dir: + args = parse_args( [ "--files", testfile, "--output-path", - output_path, - "--input-path", - input_path, + str(out_dir), "--deprecation-reason", reason, ] ) + self.assertEqual(args.files, [Path(testfile)]) + self.assertEqual(args.output_path, out_dir) + self.assertEqual(args.deprecation_reason, reason) + + def test_mandatory_arg_group_both(self): + testfile = "testfile.nasl" + reason = "NOTUS" + + with ( + TemporaryDirectory() as out_dir, + TemporaryDirectory() as in_dir, + ): + with self.assertRaises(SystemExit): + parse_args( + [ + "--files", + testfile, + "--input-path", + str(in_dir), + "--deprecation-reason", + reason, + "--output-path", + str(out_dir), + ] + ) def test_invalid_reason(self): - output_path = "attic/" - input_path = "nasl/common" reason = "foo" - with self.assertRaises(SystemExit): - parse_args( - [ - "--output-path", - output_path, - "--input-path", - input_path, - "--deprecation-reason", - reason, - ] - ) + + with TemporaryDirectory() as out_dir, TemporaryDirectory() as in_dir: + with self.assertRaises(SystemExit): + parse_args( + [ + "--output-path", + str(out_dir), + "--input-path", + str(in_dir), + "--deprecation-reason", + reason, + ] + ) def test_mandatory_arg_group_neither(self): - output_path = "attic/" reason = "NOTUS" - with self.assertRaises(SystemExit): - parse_args( - [ - "--output-path", - output_path, - "--deprecation-reason", - reason, - ] - ) + with TemporaryDirectory() as out_dir: + with self.assertRaises(SystemExit): + parse_args( + [ + "--output-path", + str(out_dir), + "--deprecation-reason", + reason, + ] + ) NASL_CONTENT = ( From 6348a3f424ff9b4fda63472fe687470296b36ec0 Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Tue, 9 Jul 2024 17:39:35 +0200 Subject: [PATCH 11/12] Change: enable running with no config if no plugin needs a config --- troubadix/argparser.py | 15 +++++++++------ troubadix/runner.py | 36 +++++++++++++++++++++++++++++++++--- troubadix/troubadix.py | 18 +++++------------- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index da8a9343..c8f1d6f2 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -235,15 +235,18 @@ def parse_args( help="Don't print the statistic", ) - parser.add_argument( + config_group = parser.add_mutually_exclusive_group() + config_group.add_argument( "-c", "--config", - type=file_type, + type=Path, default="troubadix.toml", - help=( - "Specify the path to the file that contains additional " - "configuration for the plugins" - ), + help=("Path to the configuration file (default: config.toml)"), + ) + config_group.add_argument( + "--no-config", + action="store_true", + help="Run without a configuration file", ) if not args: diff --git a/troubadix/runner.py b/troubadix/runner.py index 126fb8b4..9b425168 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -17,9 +17,10 @@ import datetime import signal +import sys from multiprocessing import Pool from pathlib import Path -from typing import Iterable +from typing import Iterable, Optional from troubadix.helper.patterns import ( init_script_tag_patterns, @@ -30,6 +31,11 @@ from troubadix.reporter import Reporter from troubadix.results import FileResults, Results +try: + import tomllib +except ImportError: + import tomli as tomllib + CHUNKSIZE = 1 # default 1 @@ -53,7 +59,7 @@ def __init__( included_plugins: Iterable[str] = None, fix: bool = False, ignore_warnings: bool = False, - plugins_config: dict, + plugins_config_path: Optional[Path], ) -> None: # plugins initialization self.plugins = StandardPlugins(excluded_plugins, included_plugins) @@ -61,7 +67,25 @@ def __init__( self._excluded_plugins = excluded_plugins self._included_plugins = included_plugins - self.plugins_config = plugins_config + self.requires_config = self._check_requires_config() + if self.requires_config: + if plugins_config_path is None: + print( + "Plugins are being run that require a external config file" + ) + sys.exit(1) + + # Get the plugins configurations from the external toml file + try: + with open(plugins_config_path, "rb") as file: + self.plugins_config = tomllib.load(file) + except FileNotFoundError: + print(f"Config file '{plugins_config_path}' does not exist") + sys.exit(1) + except tomllib.TOMLDecodeError as e: + print(f"Error decoding TOML file '{plugins_config_path}': {e}") + sys.exit(1) + self._reporter = reporter self._n_jobs = n_jobs self._root = root @@ -110,6 +134,12 @@ def _initialize_plugins(self, context, plugin_classes): for plugin_class in plugin_classes ] + def _check_requires_config(self): + return any( + plugin.require_external_config + for plugin in self.plugins.files_plugins + self.plugins.file_plugins + ) + def _run_pooled(self, files: Iterable[Path]): """Run all plugins that check single files""" self._reporter.set_files_count(len(files)) diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index 46762002..665d8934 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -30,11 +30,6 @@ from troubadix.reporter import Reporter from troubadix.runner import Runner -try: - import tomllib -except ImportError: - import tomli as tomllib - def generate_file_list( dirs: Iterable[Path], @@ -168,13 +163,10 @@ def main(args=None): first_file = files[0].resolve() root = get_root(first_file) - # Get the plugins configurations from the external toml file - try: - with open(parsed_args.config, "rb") as file: - plugins_config = tomllib.load(file) - except FileNotFoundError: - term.warning(f"Config file '{parsed_args.config}' does not exist") - sys.exit(1) + if parsed_args.no_config: + config_path = None + else: + config_path = parsed_args.config reporter = Reporter( term=term, @@ -195,7 +187,7 @@ def main(args=None): fix=parsed_args.fix, ignore_warnings=parsed_args.ignore_warnings, root=root, - plugins_config=plugins_config, + plugins_config_path=config_path, ) term.info(f"Start linting {len(files)} files ... ") From dab322cde9233623504d8ce429bc8548ff1bb5cf Mon Sep 17 00:00:00 2001 From: Niklas Hargarter Date: Wed, 10 Jul 2024 15:53:12 +0200 Subject: [PATCH 12/12] Change: Remove unnecessary command line flag --- troubadix/argparser.py | 13 +++++-------- troubadix/runner.py | 9 ++------- troubadix/troubadix.py | 7 +------ 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/troubadix/argparser.py b/troubadix/argparser.py index c8f1d6f2..27f744cf 100644 --- a/troubadix/argparser.py +++ b/troubadix/argparser.py @@ -235,18 +235,15 @@ def parse_args( help="Don't print the statistic", ) - config_group = parser.add_mutually_exclusive_group() - config_group.add_argument( + parser.add_argument( "-c", "--config", type=Path, default="troubadix.toml", - help=("Path to the configuration file (default: config.toml)"), - ) - config_group.add_argument( - "--no-config", - action="store_true", - help="Run without a configuration file", + help=( + "Specify the path to the file that contains additional " + "configuration for the plugins" + ), ) if not args: diff --git a/troubadix/runner.py b/troubadix/runner.py index 9b425168..e63bef4a 100644 --- a/troubadix/runner.py +++ b/troubadix/runner.py @@ -20,7 +20,7 @@ import sys from multiprocessing import Pool from pathlib import Path -from typing import Iterable, Optional +from typing import Iterable from troubadix.helper.patterns import ( init_script_tag_patterns, @@ -59,7 +59,7 @@ def __init__( included_plugins: Iterable[str] = None, fix: bool = False, ignore_warnings: bool = False, - plugins_config_path: Optional[Path], + plugins_config_path: Path, ) -> None: # plugins initialization self.plugins = StandardPlugins(excluded_plugins, included_plugins) @@ -69,11 +69,6 @@ def __init__( self.requires_config = self._check_requires_config() if self.requires_config: - if plugins_config_path is None: - print( - "Plugins are being run that require a external config file" - ) - sys.exit(1) # Get the plugins configurations from the external toml file try: diff --git a/troubadix/troubadix.py b/troubadix/troubadix.py index 665d8934..a6a6cd09 100644 --- a/troubadix/troubadix.py +++ b/troubadix/troubadix.py @@ -163,11 +163,6 @@ def main(args=None): first_file = files[0].resolve() root = get_root(first_file) - if parsed_args.no_config: - config_path = None - else: - config_path = parsed_args.config - reporter = Reporter( term=term, fix=parsed_args.fix, @@ -187,7 +182,7 @@ def main(args=None): fix=parsed_args.fix, ignore_warnings=parsed_args.ignore_warnings, root=root, - plugins_config_path=config_path, + plugins_config_path=parsed_args.config, ) term.info(f"Start linting {len(files)} files ... ")