diff --git a/.gitignore b/.gitignore index deb6f4a1..55f89a0d 100644 --- a/.gitignore +++ b/.gitignore @@ -244,5 +244,6 @@ fabric.properties *.drawio.dtmp # Secureli-generated files (do not modify): -.secureli +.secureli/logs +.secureli/repo-config.yaml # End Secureli-generated files diff --git a/.secureli.yaml b/.secureli.yaml index ee482100..542f0dc6 100644 --- a/.secureli.yaml +++ b/.secureli.yaml @@ -9,3 +9,5 @@ repo_files: - .png - .jpg max_file_size: 1000000 +telemetry: + api_url: https://log-api.newrelic.com/log/v1 diff --git a/.pre-commit-config.yaml b/.secureli/.pre-commit-config.yaml similarity index 100% rename from .pre-commit-config.yaml rename to .secureli/.pre-commit-config.yaml diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 00000000..7bf3fdc8 --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,7 @@ +{ + "recommendations": [ + "ms-python.python", + "ms-python.vscode-pylance", + "ms-python.black-formatter" + ] +} diff --git a/.vscode/launch.json b/.vscode/launch.json index 9c3074ac..39b64a3d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,7 +10,7 @@ "stopOnEntry": false, "console": "integratedTerminal", "justMyCode": true, - "args": ["--version"] + "args": ["--version", "update"] }, { "name": "Python: secureli --help", @@ -55,6 +55,15 @@ "console": "integratedTerminal", "justMyCode": true, "args": ["update"] + }, + { + "name": "Debug Tests", + "type": "debugpy", + "request": "launch", + "program": "${file}", + "purpose": ["debug-test"], + "console": "integratedTerminal", + "justMyCode": false } ] } diff --git a/pyproject.toml b/pyproject.toml index da928bbd..da9d65a6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,9 +25,9 @@ init = ["install", "secureli_init"] secureli_init = "secureli init -y" install = "poetry install" lint = "black --check ." -precommit = "pre-commit run -a" +precommit = "pre-commit run --config .secureli/.pre-commit-config.yaml --all-files" test = ["init", "lint", "coverage_run", "coverage_report"] -e2e = "bats --verbose-run tests/end-to-end/test.bats" +e2e = "bats --verbose-run tests/end-to-end" [tool.poetry.dependencies] # Until `python-dependency-injector` supports python 3.12, restrict to python 3.11 and lower diff --git a/secureli/actions/action.py b/secureli/actions/action.py index cff0c0f4..ca6d1ca9 100644 --- a/secureli/actions/action.py +++ b/secureli/actions/action.py @@ -63,16 +63,35 @@ def verify_install( ): update_config = self._update_secureli_config_only(always_yes) if update_config.outcome != VerifyOutcome.UPDATE_SUCCEEDED: - self.action_deps.echo.error(f"seCureLI could not be verified.") + self.action_deps.echo.error("seCureLI could not be verified.") return VerifyResult( outcome=update_config.outcome, ) + pre_commit_config_location = ( + self.action_deps.scanner.pre_commit.get_preferred_pre_commit_config_path( + folder_path + ) + ) + if not pre_commit_config_location.exists(): + update_result: VerifyResult = ( + self._update_secureli_pre_commit_config_location( + folder_path, always_yes + ) + ) + pre_commit_config_location = update_result.file_path + if update_result.outcome != VerifyOutcome.UPDATE_SUCCEEDED: + self.action_deps.echo.error( + "seCureLI pre-commit-config.yaml could not be updated." + ) + return update_result + config = ( secureli_config.SecureliConfig() if reset else self.action_deps.secureli_config.load() ) + languages = [] try: languages = self._detect_languages(folder_path) @@ -101,7 +120,11 @@ def verify_install( or newly_detected_languages ): return self._install_secureli( - folder_path, languages, newly_detected_languages, always_yes + folder_path, + languages, + newly_detected_languages, + always_yes, + pre_commit_config_location, ) else: self.action_deps.echo.print( @@ -121,6 +144,7 @@ def _install_secureli( detected_languages: list[str], install_languages: list[str], always_yes: bool, + pre_commit_config_location: Path = None, ) -> VerifyResult: """ Installs seCureLI into the given folder path and returns the new configuration @@ -155,7 +179,7 @@ def _install_secureli( ) language_config_result = ( self.action_deps.language_support.build_pre_commit_config( - install_languages, lint_languages + install_languages, lint_languages, pre_commit_config_location ) ) metadata = self.action_deps.language_support.apply_support( @@ -375,3 +399,42 @@ def _update_secureli_config_only(self, always_yes: bool) -> VerifyResult: return VerifyResult(outcome=VerifyOutcome.UPDATE_SUCCEEDED) except: return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED) + + def _update_secureli_pre_commit_config_location( + self, folder_path: Path, always_yes: bool + ) -> VerifyResult: + """ + In order to provide an upgrade path for existing users of secureli, + we will prompt users to move their .pre-commit-config.yaml into the .secureli/ directory. + I would consider this particular implementation to be technical debt but it is consistent with + an existing pattern for upgrading the secureli config file schema. + Once this has existed for awhile, we could remove this function altogether since + we make no promises of about backward compatibility with our pre-release versions. + Long term, I think there are better ways to implement one-time upgrade migrations + to avoid breaking backward compatibility. + """ + self.action_deps.echo.print( + "seCureLI's .pre-commit-config.yaml is in a deprecated location." + ) + response = always_yes or self.action_deps.echo.confirm( + "Would you like it automatically moved to the .secureli/ directory?", + default_response=True, + ) + if response: + try: + new_file_path = self.action_deps.scanner.pre_commit.migrate_config_file( + folder_path + ) + return VerifyResult( + outcome=VerifyOutcome.UPDATE_SUCCEEDED, file_path=new_file_path + ) + except: + return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED) + else: + self.action_deps.echo.warning(".pre-commit-config.yaml migration declined") + deprecated_location = self.action_deps.scanner.get_pre_commit_config_path( + folder_path + ) + return VerifyResult( + outcome=VerifyOutcome.UPDATE_CANCELED, file_path=deprecated_location + ) diff --git a/secureli/actions/scan.py b/secureli/actions/scan.py index 5c08db67..40527d1f 100644 --- a/secureli/actions/scan.py +++ b/secureli/actions/scan.py @@ -45,10 +45,10 @@ def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult: """ Queries repositories referenced by pre-commit hooks to check if we have the latest revisions listed in the .pre-commit-config.yaml file - :param folder_path: The folder path containing the .pre-commit-config.yaml file + :param folder_path: The folder path containing the .secureli/ folder """ - self.action_deps.echo.info("Checking for pre-commit hook updates...") + self.action_deps.echo.print("Checking for pre-commit hook updates...") pre_commit_config = self.scanner.pre_commit.get_pre_commit_config(folder_path) repos_to_update = self.scanner.pre_commit.check_for_hook_updates( @@ -56,7 +56,7 @@ def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult: ) if not repos_to_update: - self.action_deps.echo.info("No hooks to update") + self.action_deps.echo.print("No hooks to update") return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE) for repo, revs in repos_to_update.items(): diff --git a/secureli/container.py b/secureli/container.py index 5c735a69..5ef4455f 100644 --- a/secureli/container.py +++ b/secureli/container.py @@ -79,6 +79,7 @@ class Container(containers.DeclarativeContainer): pre_commit_abstraction = providers.Factory( PreCommitAbstraction, command_timeout_seconds=config.language_support.command_timeout_seconds, + echo=echo, ) # Services @@ -107,6 +108,7 @@ class Container(containers.DeclarativeContainer): git_ignore=git_ignore_service, language_config=language_config_service, data_loader=read_resource, + echo=echo, ) """Analyzes a given repo to try to identify the most common language""" diff --git a/secureli/modules/core/core_services/scanner.py b/secureli/modules/core/core_services/scanner.py index 40c4a66e..ab9e6a0b 100644 --- a/secureli/modules/core/core_services/scanner.py +++ b/secureli/modules/core/core_services/scanner.py @@ -67,7 +67,7 @@ def _parse_scan_ouput(self, folder_path: Path, output: str = "") -> ScanOuput: """ Parses the output from a scan and returns a list of Failure objects representing any hook rule failures during a scan. - :param folder_path: folder containing .pre-commit-config.yaml, usually repository root + :param folder_path: folder containing .secureli folder, usually repository root :param output: Raw output from a scan. :return: ScanOuput object representing a list of hook rule Failure objects. """ diff --git a/secureli/modules/core/core_services/updater.py b/secureli/modules/core/core_services/updater.py index 48fffef5..9ebcc202 100644 --- a/secureli/modules/core/core_services/updater.py +++ b/secureli/modules/core/core_services/updater.py @@ -65,7 +65,7 @@ def update(self, folder_path: Path = Path(".")): :param folder_path: Indicates the git folder against which you run secureli :return: ExecuteResult, indicating success or failure. """ - update_message = "Updating .pre-commit-config.yaml...\n" + update_message = "Updating pre-commit hooks...\n" output = update_message hook_install_result = self.pre_commit.update(folder_path) diff --git a/secureli/modules/language_analyzer/git_ignore.py b/secureli/modules/language_analyzer/git_ignore.py index 287ad74a..eb350a8a 100644 --- a/secureli/modules/language_analyzer/git_ignore.py +++ b/secureli/modules/language_analyzer/git_ignore.py @@ -16,7 +16,7 @@ class GitIgnoreService: """ header = "# Secureli-generated files (do not modify):" - ignore_entries = [".secureli"] + ignore_entries = [".secureli/logs", ".secureli/repo-config.yaml"] footer = "# End Secureli-generated files" def ignore_secureli_files(self): diff --git a/secureli/modules/language_analyzer/language_support.py b/secureli/modules/language_analyzer/language_support.py index 0586ad5b..1b982f46 100644 --- a/secureli/modules/language_analyzer/language_support.py +++ b/secureli/modules/language_analyzer/language_support.py @@ -7,6 +7,7 @@ import secureli.repositories.secureli_config as SecureliConfig from secureli.modules.shared.abstractions.pre_commit import PreCommitAbstraction +from secureli.modules.shared.abstractions.echo import EchoAbstraction from secureli.modules.language_analyzer import git_ignore, language_config from secureli.modules.shared.utilities import hash_config @@ -23,11 +24,13 @@ def __init__( language_config: language_config.LanguageConfigService, git_ignore: git_ignore.GitIgnoreService, data_loader: Callable[[str], str], + echo: EchoAbstraction, ): self.git_ignore = git_ignore self.pre_commit_hook = pre_commit_hook self.language_config = language_config self.data_loader = data_loader + self.echo = echo def apply_support( self, @@ -45,8 +48,8 @@ def apply_support( as well as a secret-detection hook ID, if present. """ - path_to_pre_commit_file = Path( - SecureliConfig.FOLDER_PATH / ".pre-commit-config.yaml" + path_to_pre_commit_file: Path = self.pre_commit_hook.get_pre_commit_config_path( + SecureliConfig.FOLDER_PATH ) linter_config_write_result = self._write_pre_commit_configs( @@ -129,21 +132,43 @@ def get_configuration(self, languages: list[str]) -> HookConfiguration: return HookConfiguration(repos=repos) def build_pre_commit_config( - self, languages: list[str], lint_languages: Iterable[str] + self, + languages: list[str], + lint_languages: Iterable[str], + pre_commit_config_location: Optional[Path] = None, ) -> language.BuildConfigResult: """ Builds the final .pre-commit-config.yaml from all supported repo languages. Also returns any and all linter configuration data. :param languages: list of languages to get calculated configuration for. :param lint_languages: list of languages to add lint pre-commit hooks for. - :return: BuildConfigResult + :return: language.BuildConfigResult """ - config_data = [] + config_repos = [] + existing_data = {} successful_languages: list[str] = [] linter_configs: list[LinterConfig] = [] config_languages = [*languages, "base"] config_lint_languages = [*lint_languages, "base"] + if pre_commit_config_location: + with open(pre_commit_config_location) as stream: + try: + data = yaml.safe_load(stream) + existing_data = data or {} + config_repos += data["repos"] + except yaml.YAMLError: + self.echo.error( + f"There was an issue parsing existing pre-commit-config.yaml." + ) + return language.BuildConfigResult( + successful=False, + languages_added=[], + config_data={}, + version="", + linter_configs=linter_configs, + ) + for config_language in config_languages: include_linter = config_language in config_lint_languages result = self.language_config.get_language_config( @@ -162,13 +187,13 @@ def build_pre_commit_config( else None ) data = yaml.safe_load(result.config_data) - config_data += data["repos"] or [] + config_repos += data["repos"] or [] - config = {"repos": config_data} + config = {**existing_data, "repos": config_repos} version = hash_config(yaml.dump(config)) return language.BuildConfigResult( - successful=True if len(config_data) > 0 else False, + successful=True if len(config_repos) > 0 else False, languages_added=successful_languages, config_data=config, version=version, diff --git a/secureli/modules/shared/abstractions/pre_commit.py b/secureli/modules/shared/abstractions/pre_commit.py index e435320f..964d3c65 100644 --- a/secureli/modules/shared/abstractions/pre_commit.py +++ b/secureli/modules/shared/abstractions/pre_commit.py @@ -15,6 +15,7 @@ import yaml from secureli.repositories.repo_settings import PreCommitSettings +from secureli.modules.shared.abstractions.echo import EchoAbstraction class InstallFailedError(Exception): @@ -71,8 +72,11 @@ class PreCommitAbstraction: def __init__( self, command_timeout_seconds: int, + echo: EchoAbstraction, ): self.command_timeout_seconds = command_timeout_seconds + self.CONFIG_FILE_NAME = ".pre-commit-config.yaml" + self.echo = echo def install(self, folder_path: Path) -> InstallResult: """ @@ -130,6 +134,8 @@ def execute_hooks( subprocess_args = [ "pre-commit", "run", + "--config", + self.get_pre_commit_config_path(folder_path), "--color", "always", ] @@ -216,6 +222,8 @@ def autoupdate_hooks( subprocess_args = [ "pre-commit", "autoupdate", + "--config", + self.get_pre_commit_config_path(folder_path), ] if bleeding_edge: subprocess_args.append("--bleeding-edge") @@ -257,7 +265,14 @@ def update(self, folder_path: Path) -> ExecuteResult: :param folder_path: Indicates the git folder against which you run secureli :return: ExecuteResult, indicating success or failure. """ - subprocess_args = ["pre-commit", "install-hooks", "--color", "always"] + subprocess_args = [ + "pre-commit", + "install-hooks", + "--config", + self.get_pre_commit_config_path(folder_path), + "--color", + "always", + ] completed_process = subprocess.run( subprocess_args, stdout=subprocess.PIPE, cwd=folder_path @@ -278,7 +293,14 @@ def remove_unused_hooks(self, folder_path: Path) -> ExecuteResult: :param folder_path: Indicates the git folder against which you run secureli :return: ExecuteResult, indicating success or failure. """ - subprocess_args = ["pre-commit", "gc", "--color", "always"] + subprocess_args = [ + "pre-commit", + "gc", + "--config", + self.get_pre_commit_config_path(folder_path), + "--color", + "always", + ] completed_process = subprocess.run( subprocess_args, stdout=subprocess.PIPE, cwd=folder_path @@ -291,12 +313,46 @@ def remove_unused_hooks(self, folder_path: Path) -> ExecuteResult: else: return ExecuteResult(successful=True, output=output) + def get_preferred_pre_commit_config_path(self, folder_path) -> Path: + """ + Returns the expected/non-deprecated path for .pre-commit-config.yaml. + If the file has not yet been migrated, it may not be located at this path. + """ + return folder_path / ".secureli" / self.CONFIG_FILE_NAME + + def get_pre_commit_config_path(self, folder_path: Path) -> Path: + """Returns the file path to .pre-commit-config.yaml""" + # The original location of .pre-commit-config.yaml was in the root of the repo, + # but that could conflict with existing configuration (if the repo was already using pre-commit). + # To keep seCureLI's configuration separate, we've migrated it to the .secureli/ folder. + # However, for now we still check the old path to avoid breaking existing + + ordered_config_paths = [ + self.get_preferred_pre_commit_config_path(folder_path), + folder_path / self.CONFIG_FILE_NAME, + ] + try: + return next( + ( + config_path + for config_path in ordered_config_paths + if config_path.exists() + ) + ) + except StopIteration: + raise FileNotFoundError( + f"Could not find pre-commit hooks in .secureli/{self.CONFIG_FILE_NAME}" + ) + def get_pre_commit_config(self, folder_path: Path): """ Gets the contents of the .pre-commit-config file and returns it as a dictionary :return: Dictionary containing the contents of the .pre-commit-config.yaml file """ - path_to_config = folder_path / ".pre-commit-config.yaml" + config_file_path: Path = self.get_pre_commit_config_path(folder_path) + return self._read_pre_commit_config(config_file_path) + + def _read_pre_commit_config(self, path_to_config: Path): with open(path_to_config, "r") as f: # For some reason, the mocking causes an infinite loop when we try to use yaml.safe_load() # directly on the file-like object f. Reading the contents of the file into a string as a workaround. @@ -305,6 +361,18 @@ def get_pre_commit_config(self, folder_path: Path): yaml_values = yaml.safe_load(contents) return PreCommitSettings(**yaml_values) + def migrate_config_file(self, folder_path): + """ + Feel free to delete this method after an appropriate period of time (a few months?) + """ + existing_config_file_path = self.get_pre_commit_config_path(folder_path) + new_config_file_path = folder_path / ".secureli" / self.CONFIG_FILE_NAME + self.echo.print( + f"Moving {existing_config_file_path} to {new_config_file_path}..." + ) + shutil.move(existing_config_file_path, new_config_file_path) + return new_config_file_path + def pre_commit_config_exists(self, folder_path: Path) -> bool: """ Checks if the .pre-commit-config file exists and returns a boolean diff --git a/secureli/modules/shared/models/install.py b/secureli/modules/shared/models/install.py index 43abffd8..6f91f293 100644 --- a/secureli/modules/shared/models/install.py +++ b/secureli/modules/shared/models/install.py @@ -1,5 +1,6 @@ from enum import Enum from typing import Optional +from pathlib import Path import pydantic from secureli.modules.shared.models.language import AnalyzeResult @@ -26,3 +27,4 @@ class VerifyResult(pydantic.BaseModel): outcome: VerifyOutcome config: Optional[SecureliConfig] = None analyze_result: Optional[AnalyzeResult] = None + file_path: Optional[Path] = None diff --git a/tests/actions/test_action.py b/tests/actions/test_action.py index 4a8efe12..dc3283c6 100644 --- a/tests/actions/test_action.py +++ b/tests/actions/test_action.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from secureli.modules.shared.abstractions.pre_commit import InstallResult @@ -407,6 +407,88 @@ def test_that_verify_install_returns_success_result_newly_detected_language_inst assert verify_result.outcome == VerifyOutcome.INSTALL_SUCCEEDED +def test_that_verify_install_returns_failure_result_without_re_commit_config_file_path( + action: Action, + mock_scanner: MagicMock, + mock_echo: MagicMock, +): + with (patch.object(Path, "exists", return_value=False),): + mock_scanner.pre_commit.get_preferred_pre_commit_config_path.return_value = ( + test_folder_path / ".secureli" / ".pre-commit-config.yaml" + ) + mock_scanner.pre_commit.migrate_config_file.side_effect = Exception("ERROR") + verify_result = action.verify_install( + test_folder_path, reset=False, always_yes=True + ) + mock_echo.error.assert_called_once_with( + "seCureLI pre-commit-config.yaml could not be updated." + ) + assert verify_result.outcome == VerifyOutcome.UPDATE_FAILED + + +def test_that_verify_install_continues_after_pre_commit_config_file_moved( + action: Action, + mock_scanner: MagicMock, + mock_echo: MagicMock, +): + with (patch.object(Path, "exists", return_value=False),): + mock_scanner.pre_commit.get_preferred_pre_commit_config_path.return_value = ( + test_folder_path / ".secureli" / ".pre-commit-config.yaml" + ) + verify_result = action.verify_install( + test_folder_path, reset=False, always_yes=True + ) + assert verify_result.outcome == VerifyOutcome.INSTALL_SUCCEEDED + + +def test_that_update_secureli_pre_commit_config_location_moves_file( + action: Action, + mock_scanner: MagicMock, + mock_echo: MagicMock, +): + update_file_location = test_folder_path / ".secureli" / ".pre-commit-config.yaml" + update_result = action._update_secureli_pre_commit_config_location( + update_file_location, True + ) + mock_echo.print.assert_called_once_with( + "seCureLI's .pre-commit-config.yaml is in a deprecated location." + ) + mock_scanner.pre_commit.migrate_config_file.assert_called_with(update_file_location) + assert update_result.outcome == VerifyOutcome.UPDATE_SUCCEEDED + + +def test_that_update_secureli_pre_commit_config_fails_on_exception( + action: Action, + mock_scanner: MagicMock, +): + with pytest.raises(Exception): + mock_scanner.pre_commit.get_preferred_pre_commit_config_path.return_value = ( + test_folder_path / ".secureli" / ".pre-commit-config.yaml" + ) + update_result = action._update_secureli_pre_commit_config_location( + test_folder_path, True + ) + assert update_result.outcome == VerifyOutcome.UPDATE_FAILED + + +def test_that_update_secureli_pre_commit_config_location_cancels_on_user_response( + action: Action, + mock_echo: MagicMock, + mock_scanner: MagicMock, +): + mock_echo.confirm.return_value = False + update_file_location = test_folder_path / ".secureli" / ".pre-commit-config.yaml" + update_result = action._update_secureli_pre_commit_config_location( + update_file_location, False + ) + + mock_echo.warning.assert_called_once_with( + ".pre-commit-config.yaml migration declined" + ) + mock_scanner.pre_commit.migrate_config_file.assert_not_called() + assert update_result.outcome == VerifyOutcome.UPDATE_CANCELED + + def test_that_initialize_repo_install_flow_warns_about_overwriting_pre_commit_file( action: Action, mock_language_analyzer: MagicMock, diff --git a/tests/actions/test_scan_action.py b/tests/actions/test_scan_action.py index 88c78f82..2e107be3 100644 --- a/tests/actions/test_scan_action.py +++ b/tests/actions/test_scan_action.py @@ -13,7 +13,7 @@ from secureli.repositories.secureli_config import SecureliConfig, VerifyConfigOutcome from secureli.repositories import repo_settings from unittest import mock -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from pytest_mock import MockerFixture import os @@ -209,13 +209,15 @@ def test_that_scan_repo_does_not_scan_if_not_installed( mock_scanner: MagicMock, mock_secureli_config: MagicMock, mock_echo: MagicMock, + mock_language_analyzer: MagicMock, ): - mock_secureli_config.load.return_value = SecureliConfig() - mock_echo.confirm.return_value = False + with patch.object(Path, "exists", return_value=False): + mock_secureli_config.load.return_value = SecureliConfig() + mock_secureli_config.verify.return_value = VerifyConfigOutcome.UP_TO_DATE + mock_echo.confirm.return_value = False - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - mock_scanner.scan_repo.assert_not_called() + scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) + mock_scanner.scan_repo.assert_not_called() def test_that_scan_checks_for_updates( diff --git a/tests/application/test_main.py b/tests/application/test_main.py index 7832a4b6..a16d7c0e 100644 --- a/tests/application/test_main.py +++ b/tests/application/test_main.py @@ -75,6 +75,19 @@ def test_that_version_callback_does_not_return_hook_versions_if_no_config( assert "\nHook Versions:" not in result.stdout +@pytest.mark.parametrize("test_input", ["-v", "--version"]) +def test_that_version_callback_returns_hook_versions_if_config( + test_input: str, +): + with patch.object(Path, "exists", return_value=True): + result = CliRunner().invoke(secureli.main.app, [test_input]) + + assert result.exit_code is 0 + assert secureli_version() in result.stdout + assert "\nHook Versions:" in result.stdout + assert "--------------" in result.stdout + + def test_that_app_ignores_version_callback(mock_container: MagicMock): result = CliRunner().invoke(secureli.main.app, ["scan"]) diff --git a/tests/end-to-end/testpreservehooks.bats b/tests/end-to-end/testpreservehooks.bats new file mode 100644 index 00000000..13653bd8 --- /dev/null +++ b/tests/end-to-end/testpreservehooks.bats @@ -0,0 +1,31 @@ +# Test to ensure pre-exisiting hooks within the .pre-commit-config.yaml files +# are persisted when installing secureli + +MOCK_REPO='tests/test-data/mock-repo' + +setup() { + load "${BATS_LIBS_ROOT}/bats-support/load" + load "${BATS_LIBS_ROOT}/bats-assert/load" + mkdir -p $MOCK_REPO + echo '# Existing YAML Contents should persist' > $MOCK_REPO/.pre-commit-config.yaml + echo 'repos:' >> $MOCK_REPO/.pre-commit-config.yaml + echo '- repo: https://github.com/hhatto/autopep8' >> $MOCK_REPO/.pre-commit-config.yaml + echo ' rev: v2.0.4' >> $MOCK_REPO/.pre-commit-config.yaml + echo ' hooks:' >> $MOCK_REPO/.pre-commit-config.yaml + echo ' - id: autopep8' >> $MOCK_REPO/.pre-commit-config.yaml + echo 'fail_fast: false' >> $MOCK_REPO/.pre-commit-config.yaml + echo 'print("hello world!")' > $MOCK_REPO/hw.py + run git init $MOCK_REPO +} + +@test "can preserve pre-existing hooks" { + run python secureli/main.py init -y --directory $MOCK_REPO + run grep 'https://github.com/hhatto/autopep8' $MOCK_REPO/.secureli/.pre-commit-config.yaml + assert_output --partial 'https://github.com/hhatto/autopep8' + run grep 'fail_fast: false' $MOCK_REPO/.secureli/.pre-commit-config.yaml + assert_output --partial 'fail_fast: false' +} + +teardown() { + rm -rf $MOCK_REPO +} diff --git a/tests/modules/language_analyzer/test_git_ignore.py b/tests/modules/language_analyzer/test_git_ignore.py index 05a3cd93..63c31474 100644 --- a/tests/modules/language_analyzer/test_git_ignore.py +++ b/tests/modules/language_analyzer/test_git_ignore.py @@ -112,7 +112,7 @@ def test_that_git_ignore_updates_existing_file_if_block_is_present( 0 ] assert args[0].find("# existing contents") == 0 # still starts with header - assert args[0].find(".secureli") != -1 # .secureli folder now added + assert args[0].find(".secureli/logs") != -1 # .secureli folder now added assert args[0].find("...") == -1 # initial data now missing diff --git a/tests/modules/language_analyzer/test_language_support.py b/tests/modules/language_analyzer/test_language_support.py index 867185fb..0c558c82 100644 --- a/tests/modules/language_analyzer/test_language_support.py +++ b/tests/modules/language_analyzer/test_language_support.py @@ -1,14 +1,18 @@ -from unittest.mock import MagicMock - import pytest +import yaml + from _pytest.python_api import raises from pytest_mock import MockerFixture +from unittest.mock import MagicMock, patch +from pathlib import Path from secureli.modules.shared.abstractions import pre_commit from secureli.modules.language_analyzer import language_support, language_config from secureli.modules.shared.models.config import LinterConfig, LinterConfigData from secureli.modules.shared.models import language +test_folder_path = Path("does-not-matter") + @pytest.fixture() def mock_open_config(mocker: MockerFixture): @@ -93,12 +97,14 @@ def language_support_service( mock_git_ignore: MagicMock, mock_language_config_service: MagicMock, mock_data_loader: MagicMock, + mock_echo: MagicMock, ) -> language_support.LanguageSupportService: return language_support.LanguageSupportService( pre_commit_hook=mock_pre_commit_hook, git_ignore=mock_git_ignore, language_config=mock_language_config_service, data_loader=mock_data_loader, + echo=mock_echo, ) @@ -394,6 +400,79 @@ def test_write_pre_commit_configs_writes_successfully( assert mock_open.return_value.write.call_count == len(configs) +def test_build_pre_commit_displays_error_parsing_existing_config( + language_support_service: language_support.LanguageSupportService, + mock_language_config_service: MagicMock, + mock_open: MagicMock, + mock_echo: MagicMock, +): + with ( + patch("builtins.open", mock_open(read_data="data")), + patch.object(yaml, "safe_load", side_effect=yaml.YAMLError), + ): + mock_language_config_service.get_language_config.return_value = language.LanguagePreCommitResult( + language="Python", + version="abc123", + linter_config=language.LoadLinterConfigsResult( + successful=True, + linter_data=[{"filename": "test.txt", "settings": {}}], + ), + config_data=""" + repos: + """, + ) + mock_data_loader.return_value = "" + languages = ["RadLang"] + lint_languages = [*languages] + + result = language_support_service.build_pre_commit_config( + languages, lint_languages, test_folder_path + ) + + mock_echo.error.assert_called_with( + "There was an issue parsing existing pre-commit-config.yaml." + ) + assert result.successful == False + + +def test_build_pre_commit_respects_existing_pre_commit_config( + language_support_service: language_support.LanguageSupportService, + mock_language_config_service: MagicMock, + mock_open: MagicMock, +): + mock_language_config_service.get_language_config.return_value = language.LanguagePreCommitResult( + language="Python", + version="abc123", + linter_config=language.LoadLinterConfigsResult( + successful=True, + linter_data=[{"filename": "test.txt", "settings": {}}], + ), + config_data=""" + repos: + """, + ) + mock_data_loader.return_value = "" + languages = ["RadLang"] + lint_languages = [*languages] + with ( + patch("builtins.open", mock_open(read_data="data")), + patch.object( + yaml, "safe_load", return_value={"repos": [{"autopep8": {"version": 0}}]} + ), + ): + result = language_support_service.build_pre_commit_config( + languages, lint_languages, test_folder_path + ) + assert result.successful == True + assert result.config_data == { + "repos": [ + {"autopep8": {"version": 0}}, + {"autopep8": {"version": 0}}, + {"autopep8": {"version": 0}}, + ] + } + + def test_write_pre_commit_configs_ignores_empty_linter_arr( language_support_service: language_support.LanguageSupportService, mock_open: MagicMock, diff --git a/tests/modules/shared/abstractions/test_pre_commit.py b/tests/modules/shared/abstractions/test_pre_commit.py index a386ebf6..4c05d777 100644 --- a/tests/modules/shared/abstractions/test_pre_commit.py +++ b/tests/modules/shared/abstractions/test_pre_commit.py @@ -1,16 +1,21 @@ import datetime import shutil + import unittest.mock as um -from pathlib import Path +from pathlib import Path, PosixPath from subprocess import CompletedProcess from unittest.mock import MagicMock import pytest from pytest_mock import MockerFixture -from secureli.modules.shared.abstractions import pre_commit +from secureli.modules.shared.abstractions.pre_commit import ( + InstallResult, + PreCommitAbstraction, +) from secureli.repositories import repo_settings + test_folder_path = Path("does-not-matter") example_git_sha = "a" * 40 @@ -71,245 +76,282 @@ def mock_subprocess(mocker: MockerFixture) -> MagicMock: return mock_subprocess +# TODO consider removing if I don't actually need to test anything? @pytest.fixture() -def mock_pre_commit( +def mock_echo(mocker: MockerFixture) -> MagicMock: + mock_echo = MagicMock() + return mock_echo + + +@pytest.fixture() +def pre_commit( + mock_echo: mock_echo, mock_hashlib: MagicMock, mock_open: MagicMock, mock_subprocess: MagicMock, -) -> pre_commit.PreCommitAbstraction: - return pre_commit.PreCommitAbstraction( - command_timeout_seconds=300, - ) +) -> PreCommitAbstraction: + return PreCommitAbstraction(command_timeout_seconds=300, echo=mock_echo) def test_that_pre_commit_executes_hooks_successfully( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.execute_hooks(test_folder_path) + 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.execute_hooks(test_folder_path) - assert execute_result.successful - assert "--all-files" not in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--all-files" not in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_executes_hooks_successfully_including_all_files( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.execute_hooks(test_folder_path, all_files=True) + 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.execute_hooks(test_folder_path, all_files=True) - assert execute_result.successful - assert "--all-files" in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--all-files" in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_executes_hooks_and_reports_failures( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=1) - execute_result = mock_pre_commit.execute_hooks(test_folder_path) + 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.execute_hooks(test_folder_path) - assert not execute_result.successful + assert not execute_result.successful def test_that_pre_commit_executes_a_single_hook_if_specified( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - mock_pre_commit.execute_hooks(test_folder_path, hook_id="detect-secrets") + with (um.patch.object(Path, "exists") as mock_exists,): + mock_exists.return_value = True + mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) + pre_commit.execute_hooks(test_folder_path, hook_id="detect-secrets") - assert mock_subprocess.run.call_args_list[0].args[0][-1] == "detect-secrets" + assert mock_subprocess.run.call_args_list[0].args[0][-1] == "detect-secrets" def test_that_pre_commit_executes_hooks_on_specified_files( - mock_pre_commit: pre_commit.PreCommitAbstraction, mock_subprocess: MagicMock + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock ): + with (um.patch.object(Path, "exists") as mock_exists,): + files = ["test_file.py", "test-file.js"] + mock_subprocess.return_value = CompletedProcess(args=[], returncode=0) + pre_commit.execute_hooks( + test_folder_path, + hook_id="detect-secrets", + files=files, + ) - files = ["test_file.py", "test-file.js"] - mock_subprocess.return_value = CompletedProcess(args=[], returncode=0) - mock_pre_commit.execute_hooks( - test_folder_path, - hook_id="detect-secrets", - files=files, - ) - - sub_process_args: list[str] = mock_subprocess.run.call_args_list[0].args[0] - files_arg_idx = sub_process_args.index("--files") + sub_process_args: [str] = mock_subprocess.run.call_args_list[0].args[0] + files_arg_idx = sub_process_args.index("--files") - assert " ".join(files) == sub_process_args[files_arg_idx + 1] + assert " ".join(files) == sub_process_args[files_arg_idx + 1] def test_that_pre_commit_does_not_execute_hooks_on_specified_files_if_not_included( - mock_pre_commit: pre_commit.PreCommitAbstraction, mock_subprocess: MagicMock + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock ): - - mock_subprocess.return_value = CompletedProcess(args=[], returncode=0) - mock_pre_commit.execute_hooks( - test_folder_path, - hook_id="detect-secrets", - ) - sub_process_args: list[str] = mock_subprocess.run.call_args_list[0].args[0] - assert "--files" not in sub_process_args + with (um.patch.object(Path, "exists") as mock_exists,): + mock_subprocess.return_value = CompletedProcess(args=[], returncode=0) + pre_commit.execute_hooks( + test_folder_path, + hook_id="detect-secrets", + ) + sub_process_args: [str] = mock_subprocess.run.call_args_list[0].args[0] + assert "--files" not in sub_process_args ##### autoupdate_hooks ##### def test_that_pre_commit_autoupdate_hooks_executes_successfully( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks(test_folder_path) + 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) - assert execute_result.successful + assert execute_result.successful def test_that_pre_commit_autoupdate_hooks_properly_handles_failed_executions( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=1) - execute_result = mock_pre_commit.autoupdate_hooks(test_folder_path) + 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) - assert not execute_result.successful + assert not execute_result.successful def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_bleeding_edge( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks( - test_folder_path, bleeding_edge=True - ) + 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, bleeding_edge=True + ) - assert execute_result.successful - assert "--bleeding-edge" in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--bleeding-edge" in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_freeze( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks(test_folder_path, freeze=True) + 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) - assert execute_result.successful - assert "--freeze" in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--freeze" in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_autoupdate_hooks_executes_successfully_with_repos( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): test_repos = ["some-repo-url"] - mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks( - test_folder_path, repos=test_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) - assert execute_result.successful - assert "--repo some-repo-url" in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--repo 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( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): test_repos = ["some-repo-url", "some-other-repo-url"] mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks( - test_folder_path, repos=test_repos - ) + 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) - 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 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] + ) def test_that_pre_commit_autoupdate_hooks_fails_with_repos_containing_non_strings( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): test_repos = [{"something": "something-else"}] mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks( - test_folder_path, repos=test_repos - ) + 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) - assert not execute_result.successful + assert not execute_result.successful def test_that_pre_commit_autoupdate_hooks_ignores_repos_when_repos_is_a_dict( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): test_repos = {} mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) # type: ignore + 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 - assert execute_result.successful - assert "--repo {}" not in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--repo {}" not in mock_subprocess.run.call_args_list[0].args[0] def test_that_pre_commit_autoupdate_hooks_converts_repos_when_repos_is_a_string( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): test_repos = "string" mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.autoupdate_hooks(test_folder_path, repos=test_repos) # type: ignore + 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 - assert execute_result.successful - assert "--repo string" in mock_subprocess.run.call_args_list[0].args[0] + assert execute_result.successful + assert "--repo string" in mock_subprocess.run.call_args_list[0].args[0] ##### update ##### def test_that_pre_commit_update_executes_successfully( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.update(test_folder_path) + with (um.patch.object(Path, "exists") as mock_exists,): + mock_exists.return_value = True + execute_result = pre_commit.update(test_folder_path) - assert execute_result.successful + assert execute_result.successful def test_that_pre_commit_update_properly_handles_failed_executions( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=1) - execute_result = mock_pre_commit.update(test_folder_path) + with (um.patch.object(Path, "exists") as mock_exists,): + mock_exists.return_value = True + execute_result = pre_commit.update(test_folder_path) - assert not execute_result.successful + assert not execute_result.successful ##### remove_unused_hooks ##### def test_that_pre_commit_remove_unused_hookss_executes_successfully( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=0) - execute_result = mock_pre_commit.remove_unused_hooks(test_folder_path) + with (um.patch.object(Path, "exists") as mock_exists,): + mock_exists.return_value = True + execute_result = pre_commit.remove_unused_hooks(test_folder_path) - assert execute_result.successful + assert execute_result.successful def test_that_pre_commit_remove_unused_hooks_properly_handles_failed_executions( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, mock_subprocess: MagicMock, ): mock_subprocess.run.return_value = CompletedProcess(args=[], returncode=1) - execute_result = mock_pre_commit.remove_unused_hooks(test_folder_path) + with (um.patch.object(Path, "exists") as mock_exists,): + mock_exists.return_value = True + execute_result = pre_commit.remove_unused_hooks(test_folder_path) - assert not execute_result.successful + assert not execute_result.successful def test_that_pre_commit_install_creates_pre_commit_hook_for_secureli( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): with ( um.patch("builtins.open", um.mock_open()) as mock_open, @@ -324,18 +366,16 @@ def test_that_pre_commit_install_creates_pre_commit_hook_for_secureli( mock_is_file.return_value = False - result = mock_pre_commit.install(test_folder_path) + result = pre_commit.install(test_folder_path) - assert result == pre_commit.InstallResult( - successful=True, backup_hook_path=None - ) + assert result == InstallResult(successful=True, backup_hook_path=None) mock_open.assert_called_once() mock_chmod.assert_called_once() mock_copy.assert_not_called() def test_that_pre_commit_install_creates_backup_file_when_already_exists( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): mock_backup_datetime = datetime.datetime(2024, 1, 1, 6, 30, 45) with ( @@ -349,7 +389,7 @@ def test_that_pre_commit_install_creates_backup_file_when_already_exists( mock_is_file.return_value = True mock_dt.now.return_value = mock_backup_datetime - result = mock_pre_commit.install(test_folder_path) + result = pre_commit.install(test_folder_path) assert result.successful == True assert ( @@ -360,9 +400,13 @@ def test_that_pre_commit_install_creates_backup_file_when_already_exists( def test_pre_commit_config_file_is_deserialized_correctly( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): - with um.patch("builtins.open", um.mock_open()) as mock_open: + with ( + um.patch("builtins.open", um.mock_open()) as mock_open, + um.patch.object(Path, "exists") as mock_exists, + ): + mock_exists.return_value = True mock_open.return_value.read.return_value = ( "repos:\n" " - repo: my-repo\n" @@ -371,7 +415,7 @@ def test_pre_commit_config_file_is_deserialized_correctly( " - id: detect-secrets\n" " args: ['--foo', '--bar']\n" ) - pre_commit_config = mock_pre_commit.get_pre_commit_config(test_folder_path) + pre_commit_config = pre_commit.get_pre_commit_config(test_folder_path) assert pre_commit_config.repos[0].url == "my-repo" assert pre_commit_config.repos[0].rev == "tag1" assert pre_commit_config.repos[0].hooks[0].id == "detect-secrets" @@ -381,7 +425,7 @@ def test_pre_commit_config_file_is_deserialized_correctly( argnames=["rev", "rev_is_sha"], argvalues=[("tag1", False), (example_git_sha, True)] ) def test_check_for_hook_updates_infers_freeze_param_when_not_provided( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, rev: str, rev_is_sha: bool, ): @@ -399,12 +443,12 @@ def test_check_for_hook_updates_infers_freeze_param_when_not_provided( 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 # Returning the same revision info on update means the hook will be considered up to date - mock_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) def test_check_for_hook_updates_respects_freeze_param_when_false( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): """ When freeze is explicitly provided, the rev_info.update() method respect that value @@ -424,12 +468,12 @@ def test_check_for_hook_updates_respects_freeze_param_when_false( 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 # Returning the same revision info on update means the hook will be considered up to date - mock_pre_commit.check_for_hook_updates(pre_commit_config, freeze=False) + pre_commit.check_for_hook_updates(pre_commit_config, freeze=False) rev_info_mock.update.assert_called_with(tags_only=True, freeze=False) def test_check_for_hook_updates_respects_freeze_param_when_true( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): with um.patch( "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo.from_config" @@ -445,12 +489,12 @@ def test_check_for_hook_updates_respects_freeze_param_when_true( 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 # Returning the same revision info on update means the hook will be considered up to date - mock_pre_commit.check_for_hook_updates(pre_commit_config, freeze=True) + 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_returns_repos_with_new_revs( - mock_pre_commit: pre_commit.PreCommitAbstraction, + pre_commit: PreCommitAbstraction, ): with um.patch( "secureli.modules.shared.abstractions.pre_commit.HookRepoRevInfo" @@ -478,21 +522,52 @@ def test_check_for_hook_updates_returns_repos_with_new_revs( repo_2_old_rev_mock.update.return_value = ( repo_2_old_rev_mock # this update should return the same rev info ) - updated_repos = mock_pre_commit.check_for_hook_updates(pre_commit_config) + updated_repos = pre_commit.check_for_hook_updates(pre_commit_config) assert len(updated_repos) == 1 # only the first repo should be returned assert updated_repos[repo_urls[0]].oldRev == "tag1" assert updated_repos[repo_urls[0]].newRev == "tag2" -def test_pre_commit_config_exists(mock_pre_commit: pre_commit.PreCommitAbstraction): +def test_pre_commit_config_exists(pre_commit: PreCommitAbstraction): with um.patch.object(Path, "exists", return_value=True): - pre_commit_exists = mock_pre_commit.pre_commit_config_exists(test_folder_path) + pre_commit_exists = pre_commit.pre_commit_config_exists(test_folder_path) assert pre_commit_exists == True -def test_pre_commit_config_does_not_exist( - mock_pre_commit: pre_commit.PreCommitAbstraction, -): +def test_pre_commit_config_does_not_exist(pre_commit: PreCommitAbstraction): with um.patch.object(Path, "exists", return_value=False): - pre_commit_exists = mock_pre_commit.pre_commit_config_exists(test_folder_path) + pre_commit_exists = pre_commit.pre_commit_config_exists(test_folder_path) assert pre_commit_exists == False + + +def test_get_pre_commit_config_path_returns_correct_location( + pre_commit: PreCommitAbstraction, +): + with um.patch.object(Path, "exists", return_value=True): + pre_commit_config_path = pre_commit.get_pre_commit_config_path(test_folder_path) + assert pre_commit_config_path == PosixPath( + f"{test_folder_path}/.secureli/.pre-commit-config.yaml" + ) + + +def test_get_pre_commit_config_path_errors_without_file( + pre_commit: PreCommitAbstraction, +): + with pytest.raises(FileNotFoundError): + pre_commit.get_pre_commit_config_path(test_folder_path) + + +def test_migrate_config_file_moves_pre_commit_conig( + pre_commit: PreCommitAbstraction, mock_echo: MagicMock +): + with ( + um.patch.object(shutil, "move") as mock_move, + um.patch.object(Path, "exists", return_value=True), + ): + pre_commit.migrate_config_file(test_folder_path) + old_location = test_folder_path / ".secureli" / ".pre-commit-config.yaml" + new_location = test_folder_path / ".secureli" / ".pre-commit-config.yaml" + mock_echo.print.assert_called_once_with( + f"Moving {old_location} to {new_location}..." + ) + mock_move.assert_called_once()