Add checks for potentially dangerous trailing characters in files and excludes fields

Characters such as |, / can have unintended and potentially dangerous
effects when used in trailing position of the files and exclude fields
of pre-commit hooks.

This change adds tests to display a WARNING when user adds these
characters in some specific ways in files and exclude
fields.
This commit is contained in:
vandan revanur 2025-07-30 20:05:49 +02:00
parent c6817210b1
commit 5a0366bcb0
2 changed files with 102 additions and 0 deletions

View file

@ -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'),
)

View file

@ -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'