diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index c0f736d9..72273d39 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -282,6 +282,36 @@ class OptionalSensibleRegexAtHook(cfgv.OptionalNoDefault): ) + +class PotentiallyDangerousTrailingCharactersAtHook(cfgv.OptionalNoDefault): + def _pre_process(self, value: str) -> str: + """Normalize the field value by removing tabs, newlines, spaces, and trailing )/.""" + spaces_regex_pattern = r"\s+" + trailing_bracket_and_slash_pattern = r"\)/$" + combined_pattern = f"{spaces_regex_pattern}|{trailing_bracket_and_slash_pattern}" + return re.sub(combined_pattern, "", value.replace("\t", "").replace("\n", "").strip()) + + def check(self, dct: dict[str, Any]) -> None: + super().check(dct) + pipe_trailing_pattern = r"\|(\)|\/)?$" + slash_trailing_pattern = r"\/(\)|\/)?$" + + pre_processed_field = self._pre_process(dct.get(self.key, "")) + + if re.search(pipe_trailing_pattern, pre_processed_field): + logger.error( + f"Potentially dangerous trailing pipe pattern detected in {self.key!r} field of the hook: {dct.get('id')!r}" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ) + + if re.search(slash_trailing_pattern, pre_processed_field): + logger.error( + f"Potentially dangerous trailing slash pattern detected in {self.key!r} field of the hook: {dct.get('id')!r}" + f"This can uninteded behaviour such as the files option being rendered empty" + f"It is recommended to remove the trailing character prompted" + ) + class OptionalSensibleRegexAtTop(cfgv.OptionalNoDefault): def check(self, dct: dict[str, Any]) -> None: super().check(dct) @@ -360,6 +390,8 @@ class NotAllowed(cfgv.OptionalNoDefault): _COMMON_HOOK_WARNINGS = ( OptionalSensibleRegexAtHook('files', cfgv.check_string), OptionalSensibleRegexAtHook('exclude', cfgv.check_string), + PotentiallyDangerousTrailingCharactersAtHook('files', cfgv.check_string), + PotentiallyDangerousTrailingCharactersAtHook('exclude', cfgv.check_string), DeprecatedStagesWarning('stages'), ) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 7aa84af0..a751f10a 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -240,6 +240,76 @@ def test_validate_optional_sensible_regex_at_hook(caplog, regex, warning): assert caplog.record_tuples == [('pre_commit', logging.WARNING, warning)] + +@pytest.mark.parametrize( + ('regex', 'warning'), + ( + ( + "(?x)^(\n^some-dir/some-sub-dir|\n)/", + "Potentially dangerous trailing pipe pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "^some/path1|", + "Potentially dangerous trailing pipe pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(\n" "^some/path1|\n" "^some/path2|\n" ")", + "Potentially dangerous trailing pipe pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "^some/path2/", + "Potentially dangerous trailing slash pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(^some-dir/)/", + "Potentially dangerous trailing slash pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(\n^some-dir|)/", + "Potentially dangerous trailing pipe pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(\n^some-dir/\n)/", + "Potentially dangerous trailing slash pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(\n^some-dir/\n\t\t\t\t)/", + "Potentially dangerous trailing slash pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ( + "(?x)^(\n^some-dir/\n )/", + "Potentially dangerous trailing slash pattern detected in 'files' field of the hook: 'flake8'" + "This can uninteded behaviour such as the files option being rendered empty" + "It is recommended to remove the trailing character prompted" + ), + ), +) +def test_validate_potentially_dangerous_trailing_characters_at_hook(caplog, regex, warning): + config_obj = { + 'id': 'flake8', + 'files': regex, + } + cfgv.validate(config_obj, CONFIG_HOOK_DICT) + + assert caplog.record_tuples == [('pre_commit', logging.ERROR, warning)] + + def test_validate_optional_sensible_regex_at_local_hook(caplog): config_obj = sample_local_config() config_obj['hooks'][0]['files'] = 'dir/*.py'