diff --git a/secureli/modules/core/core_services/updater.py b/secureli/modules/core/core_services/updater.py index 120646a9..e54929a0 100644 --- a/secureli/modules/core/core_services/updater.py +++ b/secureli/modules/core/core_services/updater.py @@ -25,6 +25,7 @@ def update_hooks( bleeding_edge: bool = False, freeze: bool = False, repos: Optional[list] = None, + force_update: Optional[bool] = False, ): """ Updates the precommit hooks but executing precommit's autoupdate command. Additional info at @@ -34,10 +35,11 @@ def update_hooks( the latest tagged version (which is the default behavior) :param freeze: Set to True to store "frozen" hashes in rev instead of tag names. :param repos: Dectionary of repos to update. This is used to target specific repos instead of all repos. + :param force_update: set to True to download updates for hooks whose versions aren't out of date. False means only out-of-date repos are updated :return: ExecuteResult, indicating success or failure. """ update_result = self.pre_commit.autoupdate_hooks( - folder_path, bleeding_edge, freeze, repos + folder_path, bleeding_edge, freeze, repos, force_update ) output = update_result.output diff --git a/secureli/modules/shared/abstractions/pre_commit.py b/secureli/modules/shared/abstractions/pre_commit.py index c91fc60e..4faf3f60 100644 --- a/secureli/modules/shared/abstractions/pre_commit.py +++ b/secureli/modules/shared/abstractions/pre_commit.py @@ -1,6 +1,8 @@ import datetime from pathlib import Path import shutil +from urllib.parse import urlparse +from git import Repo # Note that this import is pulling from the pre-commit tool's internals. # A cleaner approach would be to update pre-commit @@ -189,8 +191,14 @@ def check_for_hook_updates( "repo": repo_config.url } # PreCommitSettings uses "url" instead of "repo", so we need to copy that value over old_rev_info = HookRepoRevInfo.from_config(repo_config_dict) + + # don't try and update the local repo + if old_rev_info.repo == "local": + continue + # if the revision currently specified in .pre-commit-config.yaml looks like a full git SHA # (40-character hex string), then set freeze to True + freeze = ( bool(git_commit_sha_pattern.fullmatch(repo_config.rev)) if freeze is None @@ -208,6 +216,7 @@ def autoupdate_hooks( bleeding_edge: bool = False, freeze: bool = False, repos: Optional[list] = None, + force_update: Optional[bool] = False, ) -> ExecuteResult: """ Updates the precommit hooks by executing precommit's autoupdate command. Additional info at @@ -217,8 +226,17 @@ def autoupdate_hooks( the latest tagged version (which is the default behavior) :param freeze: Set to True to store "frozen" hashes in rev instead of tag names. :param repos: List of repos (url as a string) to update. This is used to target specific repos instead of all repos. + :param force_update: set to True to download updates for hooks whose versions aren't out of date. False means only out-of-date repos are updated :return: ExecuteResult, indicating success or failure. """ + if not force_update: + repos = self._get_outdated_repos(folder_path, bleeding_edge, freeze, repos) + + # if there's no outdated repos and we're not forcing updates then there's nothing more to do + if not repos and not force_update: + output = "No changes necessary.\n" + return ExecuteResult(successful=True, output=output) + subprocess_args = [ "pre-commit", "autoupdate", @@ -240,7 +258,9 @@ def autoupdate_hooks( for repo in repos: if isinstance(repo, str): - arg = "--repo {}".format(repo) + arg = "--repo" + repo_args.append(arg) + arg = format(repo) repo_args.append(arg) else: output = "Unable to update repo, string validation failed. Repo parameter should be a dictionary of strings." @@ -390,3 +410,26 @@ def pre_commit_config_exists(self, folder_path: Path) -> bool: """ path_to_config = folder_path / ".pre-commit-config.yaml" return path_to_config.exists() + + def _get_outdated_repos( + self, + folder_path: Path, + bleeding_edge: bool = False, + freeze: bool = False, + repos: Optional[list] = None, + ) -> list: + # if no repos are specified then use the pre commit config to get a list of all possible repos to update + if not repos: + precommit_config = self.get_pre_commit_config(folder_path) + outdated_repos = self.check_for_hook_updates( + precommit_config, not bleeding_edge, freeze + ) + repos = [key for key in outdated_repos.keys()] + # Only check for updates for the specified repos + else: + outdated_repos = self.check_for_hook_updates( + PreCommitSettings(repos=repos), not bleeding_edge, freeze + ) + repos = [key for key in outdated_repos.keys()] + + return repos diff --git a/tests/modules/shared/abstractions/test_pre_commit.py b/tests/modules/shared/abstractions/test_pre_commit.py index 4cf08ac2..6467bd2f 100644 --- a/tests/modules/shared/abstractions/test_pre_commit.py +++ b/tests/modules/shared/abstractions/test_pre_commit.py @@ -180,9 +180,31 @@ def test_that_pre_commit_autoupdate_hooks_executes_successfully( pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, mock_subprocess: MagicMock, ): - with (um.patch.object(Path, "exists") as mock_exists,): - mock_exists.return_value = True + with ( + um.patch.object(Path, "exists") as mock_exists, + um.patch("builtins.open", um.mock_open()) as mock_open, + um.patch( + "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info, + ): mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) + mock_exists.return_value = True + mock_open.return_value.read.return_value = ( + "repos:\n" + " - repo: my-repo\n" + " rev: tag1\n" + " hooks:\n" + " - id: detect-secrets\n" + " args: ['--foo', '--bar']\n" + ) + pre_commit_config_repo = RepositoryModels.PreCommitRepo( + repo="http://example-repo.com/", + rev="tag1", + hooks=[RepositoryModels.PreCommitHook(id="hook-id")], + ) + rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + mock_hook_repo_rev_info.return_value = rev_info_mock + rev_info_mock.update.return_value = rev_info_mock execute_result = pre_commit.autoupdate_hooks(test_folder_path) assert execute_result.successful @@ -193,9 +215,12 @@ def test_that_pre_commit_autoupdate_hooks_properly_handles_failed_executions( mock_subprocess: MagicMock, ): with (um.patch.object(Path, "exists") as mock_exists,): - mock_exists.return_value = True mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=1) - execute_result = pre_commit.autoupdate_hooks(test_folder_path) + mock_exists.return_value = True + + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, force_update=True + ) assert not execute_result.successful @@ -205,10 +230,11 @@ def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_bleeding_ed mock_subprocess: MagicMock, ): with (um.patch.object(Path, "exists") as mock_exists,): - mock_exists.return_value = True mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) + mock_exists.return_value = True + execute_result = pre_commit.autoupdate_hooks( - test_folder_path, bleeding_edge=True + test_folder_path, bleeding_edge=True, repos=["test"], force_update=True ) assert execute_result.successful @@ -222,7 +248,9 @@ def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_freeze( with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = pre_commit.autoupdate_hooks(test_folder_path, freeze=True) + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, freeze=True, force_update=True + ) assert execute_result.successful assert "--freeze" in mock_subprocess.run.call_args_list[0].args[0] @@ -236,10 +264,13 @@ def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_repos( with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, repos=test_repos, force_update=True + ) assert execute_result.successful - assert "--repo some-repo-url" in mock_subprocess.run.call_args_list[0].args[0] + assert "--repo" in mock_subprocess.run.call_args_list[0].args[0] + assert "some-repo-url" in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_multiple_repos( @@ -250,14 +281,14 @@ def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_multiple_re mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True - execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, repos=test_repos, force_update=True + ) assert execute_result.successful - assert "--repo some-repo-url" in mock_subprocess.run.call_args_list[0].args[0] - assert ( - "--repo some-other-repo-url" - in mock_subprocess.run.call_args_list[0].args[0] - ) + assert mock_subprocess.run.call_args_list[0].args[0].count("--repo") == 2 + assert "some-repo-url" in mock_subprocess.run.call_args_list[0].args[0] + assert "some-other-repo-url" in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_autoupdate_hooks_fails_with_repos_containing_non_strings( @@ -268,7 +299,9 @@ def test_that_pre_commit_autoupdate_hooks_fails_with_repos_containing_non_string mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True - execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, repos=test_repos, force_update=True + ) assert not execute_result.successful @@ -281,7 +314,7 @@ def test_that_pre_commit_autoupdate_hooks_ignores_repos_when_repos_is_a_dict( mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True - execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) # type: ignore + execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos, force_update=True) # type: ignore assert execute_result.successful assert "--repo {}" not in mock_subprocess.run.call_args_list[0].args[0] @@ -295,10 +328,84 @@ def test_that_pre_commit_autoupdate_hooks_converts_repos_when_repos_is_a_string( mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) with (um.patch.object(Path, "exists") as mock_exists,): mock_exists.return_value = True - execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) # type: ignore + execute_result = pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos, force_update=True) # type: ignore + + assert execute_result.successful + assert "--repo" in mock_subprocess.run.call_args_list[0].args[0] + assert "string" in mock_subprocess.run.call_args_list[0].args[0] + + +def test_that_pre_commit_autoupdate_hooks_executes_successfully_when_not_forcing_updates( + pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, + mock_subprocess: MagicMock, +): + with ( + um.patch.object(Path, "exists") as mock_exists, + um.patch("builtins.open", um.mock_open()) as mock_open, + um.patch( + "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info, + ): + mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) + mock_exists.return_value = True + mock_open.return_value.read.return_value = ( + "repos:\n" + " - repo: my-repo\n" + " rev: tag1\n" + " hooks:\n" + " - id: detect-secrets\n" + " args: ['--foo', '--bar']\n" + ) + pre_commit_config_repo = RepositoryModels.PreCommitRepo( + repo="http://example-repo.com/", + rev="tag1", + hooks=[RepositoryModels.PreCommitHook(id="hook-id")], + ) + rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + mock_hook_repo_rev_info.return_value = rev_info_mock + rev_info_mock.update.return_value = rev_info_mock + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, force_update=False + ) + + assert execute_result.successful + + +def test_that_pre_commit_autoupdate_hooks_executes_successfully_when_not_forcing_updates_with_repos( + pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, + mock_subprocess: MagicMock, +): + with ( + um.patch.object(Path, "exists") as mock_exists, + um.patch("builtins.open", um.mock_open()) as mock_open, + um.patch( + "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info, + ): + mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) + mock_exists.return_value = True + mock_open.return_value.read.return_value = ( + "repos:\n" + " - repo: my-repo\n" + " rev: tag1\n" + " hooks:\n" + " - id: detect-secrets\n" + " args: ['--foo', '--bar']\n" + ) + pre_commit_config_repo = RepositoryModels.PreCommitRepo( + repo="http://example-repo.com/", + rev="tag1", + hooks=[RepositoryModels.PreCommitHook(id="hook-id")], + ) + rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + mock_hook_repo_rev_info.return_value = rev_info_mock + rev_info_mock.update.return_value = rev_info_mock + + execute_result = pre_commit.autoupdate_hooks( + test_folder_path, force_update=False, repos=[pre_commit_config_repo] + ) assert execute_result.successful - assert "--repo string" in mock_subprocess.run.call_args_list[0].args[0] ##### update ##### @@ -443,10 +550,14 @@ def test_check_for_hook_updates_infers_freeze_param_when_not_provided( pre_commit_config = RepositoryModels.PreCommitSettings( repos=[pre_commit_config_repo] ) - rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + rev_info_mock = MagicMock( + rev=pre_commit_config_repo.rev, repo="http://example-repo.com/" + ) mock_hook_repo_rev_info.return_value = rev_info_mock rev_info_mock.update.return_value = rev_info_mock # Returning the same revision info on update means the hook will be considered up to date - pre_commit.check_for_hook_updates(pre_commit_config) + pre_commit.check_for_hook_updates( + pre_commit_config, + ) rev_info_mock.update.assert_called_with(tags_only=True, freeze=rev_is_sha) @@ -468,7 +579,9 @@ def test_check_for_hook_updates_respects_freeze_param_when_false( pre_commit_config = RepositoryModels.PreCommitSettings( repos=[pre_commit_config_repo] ) - rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + rev_info_mock = MagicMock( + rev=pre_commit_config_repo.rev, repo="http://example-repo.com/" + ) mock_hook_repo_rev_info.return_value = rev_info_mock rev_info_mock.update.return_value = rev_info_mock # Returning the same revision info on update means the hook will be considered up to date pre_commit.check_for_hook_updates(pre_commit_config, freeze=False) @@ -489,7 +602,9 @@ def test_check_for_hook_updates_respects_freeze_param_when_true( pre_commit_config = RepositoryModels.PreCommitSettings( repos=[pre_commit_config_repo] ) - rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev) + rev_info_mock = MagicMock( + rev=pre_commit_config_repo.rev, repo="http://example-repo.com/" + ) mock_hook_repo_rev_info.return_value = rev_info_mock rev_info_mock.update.return_value = rev_info_mock # Returning the same revision info on update means the hook will be considered up to date pre_commit.check_for_hook_updates(pre_commit_config, freeze=True) @@ -531,6 +646,50 @@ def test_check_for_hook_updates_returns_repos_with_new_revs( assert updated_repos[repo_urls[0]].newRev == "tag2" +def test_check_for_hook_updates_updates_repos_not_named_local( + pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, +): + with um.patch( + "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info: + pre_commit_config_repo = RepositoryModels.PreCommitRepo( + repo="local", + rev="tag1", + hooks=[RepositoryModels.PreCommitHook(id="hook-id")], + ) + pre_commit_config = RepositoryModels.PreCommitSettings( + repos=[pre_commit_config_repo] + ) + rev_info_mock = MagicMock( + rev=pre_commit_config_repo.rev, repo="http://example-repo.com/" + ) + mock_hook_repo_rev_info.return_value = rev_info_mock + rev_info_mock.update.return_value = rev_info_mock # Returning the same revision info on update means the hook will be considered up to date + pre_commit.check_for_hook_updates(pre_commit_config, freeze=True) + rev_info_mock.update.assert_called_with(tags_only=True, freeze=True) + + +def test_check_for_hook_updates_does_not_update_repos_named_local( + pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, +): + with um.patch( + "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info: + pre_commit_config_repo = RepositoryModels.PreCommitRepo( + repo="local", + rev="tag1", + hooks=[RepositoryModels.PreCommitHook(id="hook-id")], + ) + pre_commit_config = RepositoryModels.PreCommitSettings( + repos=[pre_commit_config_repo] + ) + rev_info_mock = MagicMock(rev=pre_commit_config_repo.rev, repo="local") + mock_hook_repo_rev_info.return_value = rev_info_mock + rev_info_mock.update.return_value = rev_info_mock # Returning the same revision info on update means the hook will be considered up to date + pre_commit.check_for_hook_updates(pre_commit_config, freeze=True) + rev_info_mock.update.assert_not_called() + + def test_pre_commit_config_exists( pre_commit: PreCommitAbstractionModels.PreCommitAbstraction, ):