From dd5ba1ab00e61d7f58b811a0ff4377440926e5dd Mon Sep 17 00:00:00 2001 From: ran Date: Sat, 11 Apr 2026 12:17:27 +0800 Subject: [PATCH] Validate log_file paths to prevent arbitrary file writes A hook manifest can specify a `log_file` with an absolute path or path traversal sequences (e.g. `../../../etc/cron.d/malicious`), causing pre-commit to write hook output to arbitrary locations on the host filesystem via `output.write_line_b`. Reject absolute paths and paths that traverse above the working directory during manifest validation. Fixes #3655 --- pre_commit/clientlib.py | 18 +++++++++++++++++- tests/clientlib_test.py | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 51f14d26..64114b1f 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -23,6 +23,22 @@ logger = logging.getLogger('pre_commit') check_string_regex = cfgv.check_and(cfgv.check_string, cfgv.check_regex) + +def _check_log_file(val: str) -> None: + if val == '': + return + if os.path.isabs(val): + raise cfgv.ValidationError( + f'log_file must be a relative path, got absolute path: {val!r}', + ) + if os.path.normpath(val).startswith('..'): + raise cfgv.ValidationError( + f'log_file must not reference a parent directory: {val!r}', + ) + + +check_log_file = cfgv.check_and(cfgv.check_string, _check_log_file) + HOOK_TYPES = ( 'commit-msg', 'post-checkout', @@ -258,7 +274,7 @@ MANIFEST_HOOK_DICT = cfgv.Map( cfgv.Optional('pass_filenames', cfgv.check_bool, True), cfgv.Optional('description', cfgv.check_string, ''), cfgv.Optional('language_version', cfgv.check_string, C.DEFAULT), - cfgv.Optional('log_file', cfgv.check_string, ''), + cfgv.Optional('log_file', check_log_file, ''), cfgv.Optional('require_serial', cfgv.check_bool, False), StagesMigration('stages', []), cfgv.Optional('verbose', cfgv.check_bool, False), diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index 2c42b80c..67e55ae7 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -7,6 +7,7 @@ import cfgv import pytest import pre_commit.constants as C +from pre_commit.clientlib import _check_log_file from pre_commit.clientlib import check_type_tag from pre_commit.clientlib import CONFIG_HOOK_DICT from pre_commit.clientlib import CONFIG_REPO_DICT @@ -605,3 +606,22 @@ def test_manifest_v5_forward_compat(tmp_path): f'=====> pre-commit version 5 is required but version {C.VERSION} ' f'is installed. Perhaps run `pip install --upgrade pre-commit`.' ) + + +@pytest.mark.parametrize('value', ('output.log', 'logs/hook.log', '')) +def test_check_log_file_valid(value): + _check_log_file(value) + + +@pytest.mark.parametrize( + 'value', + ( + '/tmp/evil.log', + '/etc/cron.d/malicious', + '../../../etc/passwd', + '../outside.log', + ), +) +def test_check_log_file_invalid(value): + with pytest.raises(cfgv.ValidationError): + _check_log_file(value)