From fd1cfc60bf8da6a533bb11ecd6b9791fc515579f Mon Sep 17 00:00:00 2001 From: Simon Segerblom Rex Date: Thu, 14 Mar 2024 11:49:49 +0100 Subject: [PATCH] Add guard against the hooks directory being a symbolic link `pre-commit install` shouldn't mess with directories residing outside of the git repository. --- pre_commit/commands/install_uninstall.py | 21 ++++++++++++++++++++- tests/commands/install_uninstall_test.py | 8 ++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index d19e0d47..bb8bc030 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -70,6 +70,22 @@ def _install_hook_script( ) -> None: hook_path, legacy_path = _hook_paths(hook_type, git_dir=git_dir) + # If the hooks directory is a symlink we need to be careful + if os.path.islink(os.path.dirname(hook_path)): + git_dir = git_dir if git_dir is not None else git.get_git_common_dir() + # If the hooks directory links to a directory outside the + # git repo we shouldn't try to mess with it + if os.path.commonpath( + [os.path.realpath(git_dir), os.path.realpath(hook_path)] + ) != os.path.realpath(git_dir): + logger.error( + 'Cowardly refusing to install hook script to a directory ' + 'outside of the git repo.\n' + f'hint: {os.path.dirname(hook_path)} is a symbolic link ' + f'to {os.path.realpath(os.path.dirname(hook_path))}.' + ) + return 1 + os.makedirs(os.path.dirname(hook_path), exist_ok=True) # If we have an existing hook, move it to pre-commit.legacy @@ -109,6 +125,7 @@ def _install_hook_script( make_executable(hook_path) output.write_line(f'pre-commit installed at {hook_path}') + return 0 def install( @@ -128,12 +145,14 @@ def install( return 1 for hook_type in _hook_types(config_file, hook_types): - _install_hook_script( + ret = _install_hook_script( config_file, hook_type, overwrite=overwrite, skip_on_missing_config=skip_on_missing_config, git_dir=git_dir, ) + if ret != 0: + return ret if hooks: install_hooks(config_file, store) diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 9eb0e741..98a305d8 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -109,6 +109,14 @@ def test_install_hooks_dead_symlink(in_git_dir, store): assert hook.exists() +def test_install_hooks_symlink_outisde_git_repo(in_git_dir, store): + hook_dir = in_git_dir.join('.git/hooks') + hook_dir.mksymlinkto(in_git_dir.join("../hooks")) + hook = hook_dir.join('pre-commit') + assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) != 0 + assert not hook.exists() + + def test_uninstall_does_not_blow_up_when_not_there(in_git_dir): assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0