diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 8f63dcff..a4eb7d69 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -146,7 +146,7 @@ jobs: name: Integration Testing needs: [ build-test, secureli-release, secureli-publish, deploy ] if: | - always() && + always() && (needs.secureli-publish.result == 'success' || needs.secureli-publish.result == 'skipped') && (needs.deploy.result == 'success' || needs.deploy.result == 'skipped') uses: ./.github/workflows/integration_testing.yml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4e6a92c4..69149ddd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1 +1,14 @@ -repos: [] +# See https://pre-commit.com for more information +# See https://pre-commit.com/hooks.html for more hooks +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-added-large-files +- repo: https://github.com/psf/black-pre-commit-mirror + rev: 23.11.0 + hooks: + - id: black diff --git a/.secureli.yaml b/.secureli.yaml index 41456942..ee482100 100644 --- a/.secureli.yaml +++ b/.secureli.yaml @@ -1,5 +1,5 @@ echo: - level: ERROR + level: DEBUG repo_files: exclude_file_patterns: - .idea/ diff --git a/pyproject.toml b/pyproject.toml index 42809050..cc38fb15 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "secureli" -version = "0.18.0" +version = "0.19.0" description = "Secure Project Manager" authors = ["Caleb Tonn "] license = "Apache-2.0" diff --git a/secureli/abstractions/pre_commit.py b/secureli/abstractions/pre_commit.py index 7f4f6f24..71677e24 100644 --- a/secureli/abstractions/pre_commit.py +++ b/secureli/abstractions/pre_commit.py @@ -1,10 +1,18 @@ -import stat -import subprocess from pathlib import Path -from typing import Optional +# Note that this import is pulling from the pre-commit tool's internals. +# A cleaner approach would be to update pre-commit +# by implementing a dry-run option for the `autoupdate` command +from pre_commit.commands.autoupdate import RevInfo as HookRepoRevInfo +from typing import Any, Optional import pydantic +import re +import stat +import subprocess +import yaml + +from secureli.repositories.settings import PreCommitSettings class InstallFailedError(Exception): @@ -34,6 +42,16 @@ class ExecuteResult(pydantic.BaseModel): output: str +class RevisionPair(pydantic.BaseModel): + """ + Used for updating hooks. + This could alternatively be implemented as named tuple, but those can't subclass pydantic's BaseModel + """ + + oldRev: str + newRev: str + + class InstallResult(pydantic.BaseModel): """ The results of calling install @@ -105,6 +123,48 @@ def execute_hooks( else: return ExecuteResult(successful=True, output=output) + def check_for_hook_updates( + self, + config: PreCommitSettings, + tags_only: bool = True, + freeze: Optional[bool] = None, + ) -> dict[str, RevisionPair]: + """ + Call's pre-commit's undocumented/internal functions to check for updates to repositories containing hooks + :param config: A model representing the contents of the .pre-commit-config.yaml file. + See :meth:`~get_pre_commit_config` to deserialize the config file into a model. + :param tags_only: Represents whether we should check for the latest git tag or the latest git commit. + This defaults to true since anyone who cares enough to be on the "bleeding edge" (tags_only=False) can manually + update with `secureli update`. + :param freeze: This indicates whether tags names should be converted to the corresponding commit hash, + in case a tag is updated to point to a different commit in the future. If not specified, we check + the existing revision in the .pre-commit-config.yaml file to see if it looks like a commit (40-character hex string), + and infer that we should replace the commit hash with another commit hash ("freezing" the tag ref). + :returns: A dictionary of outdated with repositories, where the key is the repository URL and the RevisionPair value + indicates the old and new revisions. If the result is empty/falsy, then no updates were found. + """ + + git_commit_sha_pattern = re.compile(r"^[a-f0-9]{40}$") + + repos_to_update: dict[str, RevisionPair] = {} + for repo_config in config.repos: + repo_config_dict = repo_config.__dict__ | { + "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) + # 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 + else freeze + ) + new_rev_info = old_rev_info.update(tags_only=tags_only, freeze=freeze) + revisions = RevisionPair(oldRev=old_rev_info.rev, newRev=new_rev_info.rev) + if revisions.oldRev != revisions.newRev: + repos_to_update[old_rev_info.repo] = revisions + return repos_to_update + def autoupdate_hooks( self, folder_path: Path, @@ -113,7 +173,7 @@ def autoupdate_hooks( repos: Optional[list] = None, ) -> ExecuteResult: """ - Updates the precommit hooks but executing precommit's autoupdate command. Additional info at + Updates the precommit hooks by executing precommit's autoupdate command. Additional info at https://pre-commit.com/#pre-commit-autoupdate :param folder_path: Indicates the git folder against which you run secureli :param bleeding_edge: True if updating to the bleeding edge of the default branch instead of @@ -199,3 +259,17 @@ def remove_unused_hooks(self, folder_path: Path) -> ExecuteResult: return ExecuteResult(successful=False, output=output) else: return ExecuteResult(successful=True, output=output) + + 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" + 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. + # return PreCommitSettings(**yaml.safe_load(f)) # TODO figure out why this isn't working + contents = f.read() + yaml_values = yaml.safe_load(contents) + return PreCommitSettings(**yaml_values) diff --git a/secureli/actions/action.py b/secureli/actions/action.py index 2b4a4d8e..7735260f 100644 --- a/secureli/actions/action.py +++ b/secureli/actions/action.py @@ -2,9 +2,6 @@ from enum import Enum from pathlib import Path from typing import Optional - -import pydantic - from secureli.abstractions.echo import EchoAbstraction, Color from secureli.abstractions.pre_commit import ( InstallFailedError, @@ -21,14 +18,13 @@ from secureli.services.scanner import ScannerService, ScanMode from secureli.services.updater import UpdaterService +import pydantic + class VerifyOutcome(str, Enum): INSTALL_CANCELED = "install-canceled" INSTALL_FAILED = "install-failed" INSTALL_SUCCEEDED = "install-succeeded" - UPGRADE_CANCELED = "upgrade-canceled" - UPGRADE_SUCCEEDED = "upgrade-succeeded" - UPGRADE_FAILED = "upgrade-failed" UPDATE_CANCELED = "update-canceled" UPDATE_SUCCEEDED = "update-succeeded" UPDATE_FAILED = "update-failed" @@ -249,7 +245,8 @@ def _update_secureli(self, always_yes: bool): update_result = self.action_deps.updater.update() details = update_result.output - self.action_deps.echo.print(details) + if details: + self.action_deps.echo.print(details) if update_result.successful: return VerifyResult(outcome=VerifyOutcome.UPDATE_SUCCEEDED) @@ -259,7 +256,7 @@ def _update_secureli(self, always_yes: bool): def _update_secureli_config_only(self, always_yes: bool) -> VerifyResult: self.action_deps.echo.print("seCureLI is using an out-of-date config.") response = always_yes or self.action_deps.echo.confirm( - "Update config only now?", + "Update configuration now?", default_response=True, ) if not response: diff --git a/secureli/actions/scan.py b/secureli/actions/scan.py index 90d46923..839cb9cf 100644 --- a/secureli/actions/scan.py +++ b/secureli/actions/scan.py @@ -1,10 +1,16 @@ import json import sys from pathlib import Path +from time import time from typing import Optional from secureli.abstractions.echo import EchoAbstraction -from secureli.actions.action import VerifyOutcome, Action, ActionDependencies +from secureli.actions.action import ( + VerifyOutcome, + Action, + ActionDependencies, + VerifyResult, +) from secureli.services.logging import LoggingService, LogAction from secureli.services.scanner import ( ScanMode, @@ -12,6 +18,8 @@ ) from secureli.utilities.usage_stats import post_log, convert_failures_to_failure_count +ONE_WEEK_IN_SECONDS: int = 7 * 24 * 60 * 60 + class ScanAction(Action): """The action for the secureli `scan` command, orchestrating services and outputs results""" @@ -34,6 +42,34 @@ def __init__( self.echo = echo self.logging = logging + 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 + """ + + self.action_deps.echo.info("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( + pre_commit_config + ) + + if not repos_to_update: + self.action_deps.echo.info("No hooks to update") + return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE) + + for repo, revs in repos_to_update.items(): + self.action_deps.echo.debug( + f"Found update for {repo}: {revs.oldRev} -> {revs.newRev}" + ) + self.action_deps.echo.warning( + "You have out-of-date pre-commit hooks. Run `secureli update` to update them." + ) + # Since we don't actually perform the updates here, return an outcome of UPDATE_CANCELLED + return VerifyResult(outcome=VerifyOutcome.UPDATE_CANCELED) + def scan_repo( self, folder_path: Path, @@ -53,6 +89,14 @@ def scan_repo( """ verify_result = self.verify_install(folder_path, False, always_yes) + # Check if pre-commit hooks are up-to-date + secureli_config = self.action_deps.secureli_config.load() + now: int = int(time()) + if (secureli_config.last_hook_update_check or 0) + ONE_WEEK_IN_SECONDS < now: + self._check_secureli_hook_updates(folder_path) + secureli_config.last_hook_update_check = now + self.action_deps.secureli_config.save(secureli_config) + if verify_result.outcome in self.halting_outcomes: return diff --git a/secureli/main.py b/secureli/main.py index 4ea1942d..a91d31f5 100644 --- a/secureli/main.py +++ b/secureli/main.py @@ -62,14 +62,14 @@ def init( help="Say 'yes' to every prompt automatically without input", ), directory: Annotated[ - Optional[Path], + Path, Option( ".", "--directory", "-d", help="Run secureli against a specific directory", ), - ] = ".", + ] = Path("."), ): """ Detect languages and initialize pre-commit hooks and linters for the project @@ -100,14 +100,14 @@ def scan( help="Limit the scan to a specific hook ID from your pre-commit config", ), directory: Annotated[ - Optional[Path], + Path, Option( ".", "--directory", "-d", help="Run secureli against a specific directory", ), - ] = ".", + ] = Path("."), ): """ Performs an explicit check of the repository to detect security issues without remote logging. @@ -133,14 +133,14 @@ def update( help="Update the installed pre-commit hooks to their latest versions", ), directory: Annotated[ - Optional[Path], + Path, Option( ".", "--directory", "-d", help="Run secureli against a specific directory", ), - ] = ".", + ] = Path("."), ): """ Update linters, configuration, and all else needed to maintain a secure repository. diff --git a/secureli/repositories/secureli_config.py b/secureli/repositories/secureli_config.py index fb721223..8e7351c4 100644 --- a/secureli/repositories/secureli_config.py +++ b/secureli/repositories/secureli_config.py @@ -9,9 +9,10 @@ class SecureliConfig(BaseModel): - languages: Optional[list[str]] - lint_languages: Optional[list[str]] - version_installed: Optional[str] + languages: Optional[list[str]] = None + lint_languages: Optional[list[str]] = None + version_installed: Optional[str] = None + last_hook_update_check: Optional[int] = 0 class DeprecatedSecureliConfig(BaseModel): @@ -94,10 +95,13 @@ def update(self) -> SecureliConfig: with open(secureli_config_path, "r") as f: data = yaml.safe_load(f) old_config = DeprecatedSecureliConfig.parse_obj(data) + languages: list[str] | None = ( + [old_config.overall_language] if old_config.overall_language else None + ) return SecureliConfig( - languages=[old_config.overall_language], - lint_languages=[old_config.overall_language], + languages=languages, + lint_languages=languages, version_installed=old_config.version_installed, ) diff --git a/secureli/repositories/settings.py b/secureli/repositories/settings.py index 8aeee9d6..80d3f37d 100644 --- a/secureli/repositories/settings.py +++ b/secureli/repositories/settings.py @@ -85,7 +85,7 @@ class LanguageSupportSettings(BaseSettings): command_timeout_seconds: int = Field(default=300) -class PreCommitHook(BaseSettings): +class PreCommitHook(BaseModel): """ Hook settings for pre-commit. """ @@ -96,19 +96,24 @@ class PreCommitHook(BaseSettings): exclude_file_patterns: Optional[list[str]] = Field(default=[]) -class PreCommitRepo(BaseSettings): +class PreCommitRepo(BaseModel): """ Repo settings for pre-commit. """ - url: str + url: str = Field(alias="repo") + rev: str hooks: list[PreCommitHook] = Field(default=[]) suppressed_hook_ids: list[str] = Field(default=[]) -class PreCommitSettings(BaseSettings): +class PreCommitSettings(BaseModel): """ Various adjustments that affect how seCureLI configures the pre-commit system. + + Extends schema for .pre-commit-config.yaml file. + See for details: https://pre-commit.com/#pre-commit-configyaml---top-level + """ repos: list[PreCommitRepo] = Field(default=[]) @@ -120,8 +125,8 @@ class SecureliFile(BaseModel): Represents the contents of the .secureli.yaml file """ - repo_files: Optional[RepoFilesSettings] - echo: Optional[EchoSettings] + repo_files: Optional[RepoFilesSettings] = None + echo: Optional[EchoSettings] = None language_support: Optional[LanguageSupportSettings] = Field(default=None) diff --git a/secureli/services/language_support.py b/secureli/services/language_support.py index b1f0567e..8c35465f 100644 --- a/secureli/services/language_support.py +++ b/secureli/services/language_support.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Callable, Optional, Any +from typing import Callable, Iterable, Optional, Any import pydantic import yaml @@ -117,9 +117,9 @@ def apply_support( self, languages: list[str], language_config_result: BuildConfigResult ) -> LanguageMetadata: """ - Applies Secure Build support for the provided language + Applies Secure Build support for the provided languages :param languages: list of languages to provide support for - :raises LanguageNotSupportedError if support for the language is not provided + :raises LanguageNotSupportedError if support for any language is not provided :return: Metadata including version of the language configuration that was just installed as well as a secret-detection hook ID, if present. """ @@ -205,7 +205,7 @@ def create_repo(raw_repo: dict) -> Repo: return HookConfiguration(repos=repos) def _build_pre_commit_config( - self, languages: list[str], lint_languages: list[str] + self, languages: list[str], lint_languages: Iterable[str] ) -> BuildConfigResult: """ Builds the final .pre-commit-config.yaml from all supported repo languages. Also returns any and all diff --git a/secureli/services/scanner.py b/secureli/services/scanner.py index 9f65d5e2..2ef4db09 100644 --- a/secureli/services/scanner.py +++ b/secureli/services/scanner.py @@ -4,9 +4,9 @@ import pydantic import re -import yaml from secureli.abstractions.pre_commit import PreCommitAbstraction +from secureli.repositories.settings import PreCommitSettings class ScanMode(str, Enum): @@ -94,12 +94,15 @@ 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 output: Raw output from a scan. :return: ScanOuput object representing a list of hook rule Failure objects. """ failures = [] failure_indexes = [] - config_data = self._get_config(folder_path) + pre_commit_config: PreCommitSettings = self.pre_commit.get_pre_commit_config( + folder_path + ) # Split the output up by each line and record the index of each failure output_by_line = output.split("\n") @@ -114,7 +117,7 @@ def _parse_scan_ouput(self, folder_path: Path, output: str = "") -> ScanOuput: id = self._remove_ansi_from_string(id_with_encoding) # Retrieve repo url for failure - repo = self._find_repo_from_id(hook_id=id, config=config_data) + repo = self._find_repo_from_id(hook_id=id, config=pre_commit_config) # Capture all output lines for this failure failure_output_list = self._get_single_failure_output( @@ -143,7 +146,7 @@ def _get_single_failure_output( return failure_lines - def _find_file_names(self, failure_output_list: list[str]) -> str: + def _find_file_names(self, failure_output_list: list[str]) -> list[str]: """ Finds the file name for a hook rule failure :param failure_index: The index of the initial failure in failure_output_list @@ -174,31 +177,20 @@ def _remove_ansi_from_string(self, string: str) -> str: return clean_string - def _find_repo_from_id(self, hook_id: str, config: dict): + def _find_repo_from_id(self, hook_id: str, config: PreCommitSettings): """ Retrieves the repo URL that a hook ID belongs to and returns it :param linter_id: The hook id we want to retrieve the repo url for - :config: A dict containing the contents of the .pre-commit-config.yaml file + :config: A model of the YAML data in .pre-commit-config.yaml file :return: The repo url our hook id belongs to """ - repos = config.get("repos") - for repo in repos: - hooks = repo["hooks"] - repo = repo["repo"] + for repo in config.repos: + hooks = repo.hooks + repo_str = repo.url for hook in hooks: - if hook["id"] == hook_id: - return repo + if hook.id == hook_id: + return repo_str return OutputParseErrors.REPO_NOT_FOUND - - def _get_config(self, folder_path: Path): - """ - Gets the contents of the .pre-commit-config file and returns it as a dict - :return: Dict containing the contents of the .pre-commit-config.yaml file - """ - path_to_config = folder_path / ".pre-commit-config.yaml" - with open(path_to_config, "r") as f: - data = yaml.safe_load(f) - return data diff --git a/secureli/services/updater.py b/secureli/services/updater.py index 33649f38..347b7df6 100644 --- a/secureli/services/updater.py +++ b/secureli/services/updater.py @@ -53,13 +53,13 @@ def update_hooks( if update_result.successful and not output: output = "No changes necessary.\n" - if update_result.successful and update_result.output: + if update_result.successful and output: prune_result = self.pre_commit.remove_unused_hooks(folder_path) output = output + "\nRemoving unused environments:\n" + prune_result.output return UpdateResult(successful=update_result.successful, output=output) - def update(self, folder_path: Path): + def update(self, folder_path: Path = Path(".")): """ Updates secureli with the latest local configuration. :param folder_path: Indicates the git folder against which you run secureli diff --git a/secureli/utilities/secureli_meta.py b/secureli/utilities/secureli_meta.py index 15804961..6478683e 100644 --- a/secureli/utilities/secureli_meta.py +++ b/secureli/utilities/secureli_meta.py @@ -1,6 +1,6 @@ -import pkg_resources +from importlib.metadata import version def secureli_version() -> str: """Leverage package resources to determine the current version of secureli""" - return pkg_resources.get_distribution("secureli").version + return version("secureli") diff --git a/tests/abstractions/test_pre_commit.py b/tests/abstractions/test_pre_commit.py index 1a901be5..44406167 100644 --- a/tests/abstractions/test_pre_commit.py +++ b/tests/abstractions/test_pre_commit.py @@ -16,6 +16,7 @@ ) test_folder_path = Path("does-not-matter") +example_git_sha = "a" * 40 @pytest.fixture() @@ -23,7 +24,8 @@ def settings_dict() -> dict: return PreCommitSettings( repos=[ PreCommitRepo( - url="http://example-repo.com/", + repo="http://example-repo.com/", + rev="master", hooks=[ PreCommitHook( id="hook-id", @@ -209,7 +211,7 @@ def test_that_pre_commit_autoupdate_hooks_ignores_repos_when_repos_is_a_dict( ): test_repos = {} 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) # type: ignore assert execute_result.successful assert "--repo {}" not in mock_subprocess.run.call_args_list[0].args[0] @@ -221,7 +223,7 @@ def test_that_pre_commit_autoupdate_hooks_converts_repos_when_repos_is_a_string( ): test_repos = "string" 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) # type: ignore assert execute_result.successful assert "--repo string" in mock_subprocess.run.call_args_list[0].args[0] @@ -284,3 +286,120 @@ def test_that_pre_commit_install_creates_pre_commit_hook_for_secureli( mock_open.assert_called_once() mock_chmod.assert_called_once() + + +def test_pre_commit_config_file_is_deserialized_correctly( + pre_commit: PreCommitAbstraction, +): + with um.patch("builtins.open", um.mock_open()) as mock_open: + 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 = 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" + + +@pytest.mark.parametrize( + argnames=["rev", "rev_is_sha"], argvalues=[("tag1", False), (example_git_sha, True)] +) +def test_check_for_hook_updates_infers_freeze_param_when_not_provided( + pre_commit: PreCommitAbstraction, + rev: str, + rev_is_sha: bool, +): + with um.patch( + "secureli.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info: + pre_commit_config_repo = PreCommitRepo( + repo="http://example-repo.com/", + rev=rev, + hooks=[PreCommitHook(id="hook-id")], + ) + pre_commit_config = PreCommitSettings(repos=[pre_commit_config_repo]) + 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 + 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( + pre_commit: PreCommitAbstraction, +): + """ + When freeze is explicitly provided, the rev_info.update() method respect that value + regardless of whether the existing rev is a tag or a commit hash. + """ + with um.patch( + "secureli.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info: + pre_commit_config_repo = PreCommitRepo( + repo="http://example-repo.com/", + rev=example_git_sha, + hooks=[PreCommitHook(id="hook-id")], + ) + pre_commit_config = PreCommitSettings(repos=[pre_commit_config_repo]) + 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 + 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( + pre_commit: PreCommitAbstraction, +): + with um.patch( + "secureli.abstractions.pre_commit.HookRepoRevInfo.from_config" + ) as mock_hook_repo_rev_info: + pre_commit_config_repo = PreCommitRepo( + repo="http://example-repo.com/", + rev="tag1", + hooks=[PreCommitHook(id="hook-id")], + ) + pre_commit_config = PreCommitSettings(repos=[pre_commit_config_repo]) + 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 + 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( + pre_commit: PreCommitAbstraction, +): + with um.patch( + "secureli.abstractions.pre_commit.HookRepoRevInfo" + ) as mock_hook_repo_rev_info: + repo_urls = ["http://example-repo.com/", "http://example-repo-2.com/"] + old_rev = "tag1" + repo_1_new_rev = "tag2" + pre_commit_config = PreCommitSettings( + repos=[ + PreCommitRepo( + repo=repo_url, rev=old_rev, hooks=[PreCommitHook(id="hook-id")] + ) + for repo_url in repo_urls + ] + ) + repo_1_old_rev_mock = MagicMock(rev=old_rev, repo=repo_urls[0]) + repo_1_new_rev_mock = MagicMock(rev=repo_1_new_rev, repo=repo_urls[0]) + repo_2_old_rev_mock = MagicMock(rev=old_rev, repo=repo_urls[1]) + mock_hook_repo_rev_info.from_config = MagicMock( + side_effect=[repo_1_old_rev_mock, repo_2_old_rev_mock] + ) + repo_1_old_rev_mock.update.return_value = repo_1_new_rev_mock + repo_2_old_rev_mock.update.return_value = ( + repo_2_old_rev_mock # this update should return the same rev info + ) + 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" diff --git a/tests/actions/test_scan_action.py b/tests/actions/test_scan_action.py index 77e710b8..ba58dd39 100644 --- a/tests/actions/test_scan_action.py +++ b/tests/actions/test_scan_action.py @@ -1,30 +1,57 @@ -import os from pathlib import Path -from unittest import mock -from unittest.mock import MagicMock - -import pytest - -from secureli.actions.action import ActionDependencies +from secureli.abstractions.pre_commit import RevisionPair +from secureli.actions.action import ActionDependencies, VerifyOutcome from secureli.actions.scan import ScanAction -from secureli.repositories.secureli_config import SecureliConfig +from secureli.repositories.secureli_config import SecureliConfig, VerifyConfigOutcome from secureli.repositories.settings import ( + PreCommitHook, + PreCommitRepo, + PreCommitSettings, SecureliFile, EchoSettings, EchoLevel, ) from secureli.services.scanner import ScanMode, ScanResult, Failure +from unittest import mock +from unittest.mock import MagicMock +from pytest_mock import MockerFixture + +import os +import pytest test_folder_path = Path("does-not-matter") @pytest.fixture() -def mock_scanner() -> MagicMock: +def mock_scanner(mock_pre_commit) -> MagicMock: mock_scanner = MagicMock() mock_scanner.scan_repo.return_value = ScanResult(successful=True, failures=[]) + mock_scanner.pre_commit = mock_pre_commit return mock_scanner +@pytest.fixture() +def mock_pre_commit() -> MagicMock: + mock_pre_commit = MagicMock() + mock_pre_commit.get_pre_commit_config.return_value = PreCommitSettings( + repos=[ + PreCommitRepo( + repo="http://example-repo.com/", + rev="master", + hooks=[ + PreCommitHook( + id="hook-id", + arguments=None, + additional_args=None, + ) + ], + ) + ] + ) + mock_pre_commit.check_for_hook_updates.return_value = {} + return mock_pre_commit + + @pytest.fixture() def mock_updater() -> MagicMock: mock_updater = MagicMock() @@ -32,33 +59,21 @@ def mock_updater() -> MagicMock: @pytest.fixture() -def mock_default_settings(mock_settings_repository: MagicMock) -> MagicMock: - mock_echo_settings = EchoSettings(EchoLevel.info) - mock_settings_file = SecureliFile(echo=mock_echo_settings) - mock_settings_repository.load.return_value = mock_settings_file - - return mock_settings_repository +def mock_get_time_near_epoch(mocker: MockerFixture) -> MagicMock: + return mocker.patch( + "secureli.actions.scan.time", return_value=1.0 + ) # 1 second after epoch @pytest.fixture() -def mock_default_settings_populated(mock_settings_repository: MagicMock) -> MagicMock: - mock_echo_settings = EchoSettings(EchoLevel.info) - mock_settings_file = SecureliFile( - echo=mock_echo_settings, - ) - mock_settings_repository.load.return_value = mock_settings_file - - return mock_settings_repository +def mock_get_time_far_from_epoch(mocker: MockerFixture) -> MagicMock: + return mocker.patch("secureli.actions.scan.time", return_value=1e6) @pytest.fixture() -def mock_default_settings_ignore_already_exists( - mock_settings_repository: MagicMock, -) -> MagicMock: - mock_echo_settings = EchoSettings(EchoLevel.info) - mock_settings_file = SecureliFile( - echo=mock_echo_settings, - ) +def mock_default_settings(mock_settings_repository: MagicMock) -> MagicMock: + mock_echo_settings = EchoSettings(level=EchoLevel.info) + mock_settings_file = SecureliFile(echo=mock_echo_settings) mock_settings_repository.load.return_value = mock_settings_file return mock_settings_repository @@ -193,3 +208,74 @@ def test_that_scan_repo_does_not_add_ignore_if_always_yes_is_true( scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, True) mock_settings_repository.save.assert_not_called() + + +def test_that_scan_checks_for_updates( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_secureli_config: MagicMock, + mock_pass_install_verification: MagicMock, +): + scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, always_yes=True) + mock_scanner.pre_commit.check_for_hook_updates.assert_called_once() + + +def test_that_scan_only_checks_for_updates_periodically( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_get_time_near_epoch: MagicMock, + mock_secureli_config: MagicMock, +): + mock_secureli_config.load.return_value = SecureliConfig() + + scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, always_yes=True) + mock_scanner.pre_commit.check_for_hook_updates.assert_not_called() + + +def test_that_scan_update_check_uses_pre_commit_config( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_secureli_config: MagicMock, +): + mock_secureli_config.load.return_value = SecureliConfig() + scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, always_yes=True) + mock_scanner.pre_commit.get_pre_commit_config.assert_called_once() + + +# Test that _check_secureli_hook_updates returns UP_TO_DATE if no hooks need updating +def test_scan_update_check_return_value_when_up_to_date( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_secureli_config: MagicMock, +): + mock_secureli_config.load.return_value = SecureliConfig() + result = scan_action._check_secureli_hook_updates(test_folder_path) + assert result.outcome == VerifyOutcome.UP_TO_DATE + + +# Test that _check_secureli_hook_updates returns UPDATE_CANCELED if hooks need updating +def test_scan_update_check_return_value_when_not_up_to_date( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_secureli_config: MagicMock, +): + mock_secureli_config.load.return_value = SecureliConfig() + mock_scanner.pre_commit.check_for_hook_updates.return_value = { + "http://example-repo.com/": RevisionPair(oldRev="old-rev", newRev="new-rev") + } + result = scan_action._check_secureli_hook_updates(test_folder_path) + assert result.outcome == VerifyOutcome.UPDATE_CANCELED + + +# Validate that scan_repo persists changes to the .secureli.yaml file after checking for hook updates +def test_that_scan_update_check_updates_last_check_time( + scan_action: ScanAction, + mock_scanner: MagicMock, + mock_get_time_far_from_epoch: MagicMock, + mock_secureli_config: MagicMock, + mock_pass_install_verification: MagicMock, +): + mock_secureli_config.verify.return_value = VerifyConfigOutcome.UP_TO_DATE + scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, always_yes=True) + mock_secureli_config.save.assert_called_once() + assert mock_secureli_config.save.call_args.args[0].last_hook_update_check == 1e6 diff --git a/tests/services/test_language_config.py b/tests/services/test_language_config.py index f7ac5718..4464af75 100644 --- a/tests/services/test_language_config.py +++ b/tests/services/test_language_config.py @@ -174,7 +174,7 @@ def test_that_calculate_combined_configuration_ignores_lint_config( assert mock_data_loader.call_count == 1 -def test_that_calculate_combined_configuration_ignores_lint_config( +def test_that_calculate_combined_configuration_returns_valid_config_if_config_file_is_empty( language_config_service: LanguageConfigService, mock_data_loader: MagicMock, ): diff --git a/tests/services/test_language_support.py b/tests/services/test_language_support.py index 1d6a555d..cf137e90 100644 --- a/tests/services/test_language_support.py +++ b/tests/services/test_language_support.py @@ -307,6 +307,7 @@ def test_that_language_support_throws_exception_when_language_config_file_cannot def test_that_language_support_handles_invalid_language_config( language_support_service: LanguageSupportService, mock_language_config_service: MagicMock, + mock_open: MagicMock, ): mock_language_config_service.get_language_config.return_value = ( LanguagePreCommitResult( diff --git a/tests/services/test_logging_service.py b/tests/services/test_logging_service.py index 6ca832e1..a873ddda 100644 --- a/tests/services/test_logging_service.py +++ b/tests/services/test_logging_service.py @@ -32,15 +32,6 @@ def mock_open(mocker: MockerFixture) -> MagicMock: return mock_open -@pytest.fixture() -def mock_open(mocker: MockerFixture) -> MagicMock: - mock_open = mocker.mock_open() - - mocker.patch("builtins.open", mock_open) - - return mock_open - - @pytest.fixture() def mock_language_support() -> MagicMock: mock_language_support = MagicMock()