From fc3c6ff70f20c3f54a71569d49bd7379d2dde889 Mon Sep 17 00:00:00 2001 From: stujfiter Date: Wed, 9 Aug 2023 14:32:19 -0500 Subject: [PATCH] feat: Removes mutation of .pre-commit-config.yaml by seCureLI (#264) This PR mostly deletes code related to mutating .pre-commit-config.yaml based on entries in .secureli.yaml. Once merged, users will be able to update .pre-commit-config.yaml directly to modify the behavior of the pre-commit hooks. --- .pre-commit-config.yaml | 47 +--- README.md | 48 +--- secureli/actions/action.py | 67 +---- secureli/actions/scan.py | 197 +-------------- secureli/container.py | 1 - secureli/repositories/settings.py | 8 +- secureli/services/language_config.py | 191 +-------------- secureli/services/language_support.py | 215 +--------------- secureli/settings.py | 4 +- tests/abstractions/test_pre_commit.py | 23 -- tests/actions/test_action.py | 95 ++------ tests/actions/test_scan_action.py | 294 +--------------------- tests/services/test_language_config.py | 312 +----------------------- tests/services/test_language_support.py | 247 ++++--------------- tests/services/test_secureli_ignore.py | 8 - 15 files changed, 98 insertions(+), 1659 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f5ca066c..4e6a92c4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,46 +1 @@ -repos: -- hooks: - - id: python-use-type-annotations - repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.9.0 -- hooks: - - args: - - --exclude - - tests/ - - --severity-level - - medium - id: bandit - repo: https://github.com/PyCQA/bandit - rev: 1.7.4 -- hooks: - - id: black - repo: https://github.com/psf/black - rev: 22.10.0 -- hooks: - - id: check-added-large-files - - id: check-ast - - id: check-docstring-first - - id: check-executables-have-shebangs - - id: check-shebang-scripts-are-executable - - id: check-merge-conflict - - id: check-toml - - id: check-json - - id: check-xml - - id: check-yaml - - id: debug-statements - - args: - - --allow-missing-credentials - id: detect-aws-credentials - - id: detect-private-key - - args: - - --pytest-test-first - id: name-tests-test - - id: trailing-whitespace - - id: end-of-file-fixer - - id: check-yaml - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.3.0 -- hooks: - - id: detect-secrets - repo: https://github.com/Yelp/detect-secrets - rev: v1.4.0 +repos: [] diff --git a/README.md b/README.md index 2ddc71a6..bb97a09f 100644 --- a/README.md +++ b/README.md @@ -40,13 +40,13 @@ pip install secureli Once installed you can see the latest documentation for seCureLI by entering the following on a command prompt: -```python +```bash % secureli --help ``` You will see a list of commands and descriptions of each. You can also pull up documentation for each command with the same pattern. For example: -```python +```bash % secureli init --help Usage: secureli init [OPTIONS] @@ -62,7 +62,7 @@ You will see a list of commands and descriptions of each. You can also pull up d When invoking these commands, you can combine the short versions into a single flag. For example, the following commands are equivalent: -```python +```bash % secureli init --reset --yes % secureli init -ry ``` @@ -72,7 +72,7 @@ After seCureLI is installed, you can use it to configure your local git reposito All you need to do is run: -```commandline +```bash % secureli init ``` @@ -110,17 +110,16 @@ seCureLI is configurable via a .secureli.yaml file present in the root of your l ### top level -| Key | Description | -| ------------------ |----------------------------------------------------------------------------------------------------------------------------------| -| `repo_files` | Affects how seCureLI will interpret the repository, both for language analysis and as it executes various linters. | -| `echo` | Adjusts how seCureLI will print information to the user. | -| `language_support` | Affects seCureLI's language analysis and support phase. | -| `pre_commit` | Enables various overrides and options for seCureLI's configuration and usage of pre-commit, the underlying code analysis system. | +| Key | Description | +|--------------------|--------------------------------------------------------------------------------------------------------------------| +| `repo_files` | Affects how seCureLI will interpret the repository, both for language analysis and as it executes various linters. | +| `echo` | Adjusts how seCureLI will print information to the user. | +| `language_support` | Affects seCureLI's language analysis and support phase. | ### repo_files | Key | Description | -| ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|---------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `max_file_size` | A number in bytes. Files over this size will not be considered during language analysis, for speed purposes. Default: 100000 | | `ignored_file_extensions` | Which file extensions not to consider during language analysis. | | `exclude_file_patterns` | Which file patterns to ignore during language analysis and code analysis execution. Use a typical file pattern you might find in a .gitignore file, such as `*.py` or `tests/`. Certain patterns you will have to wrap in double-quotes for the entry to be valid YAML. | @@ -128,32 +127,9 @@ seCureLI is configurable via a .secureli.yaml file present in the root of your l ### echo | Key | Description | -| ------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | +|---------|----------------------------------------------------------------------------------------------------------------------------------------------------| | `level` | The log level to display to the user. Defaults to ERROR, which includes `error` and `print` messages, without including warnings or info messages. | -### pre_commit - -| Key | Description | -| ------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `repos` | A set of template-based Pre-Commit Repos to configure with overrides, identified by URL. These override repo-configurations stored in the template, and attempting to modify a repo not configured into the template will have no effect. | -| `suppressed_repos` | A set of template-based Pre-Commit Repo URLs to completely remove from the final configuration. These remove repo configurations stored in the template, removing a repo not stored in the template will be ignored. | - -### pre_commit.repos - -| Key | Description | -| --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `url` | The identifying URL of the repo being leveraged by pre-commit, within which one or more hooks can be leveraged. | -| `hooks` | A set of hooks associated with the specified repository to override. See the next section for what we can configure there. | -| `suppressed_hook_ids` | A set of hook IDs to remove from the repository as configured within the template. Hook IDs not present in the template configuration will be ignored. | - -### pre_commit.repos.hooks - -| Key | Description | -| ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | The identifying string of the pre-commit hook to override. | -| `arguments` | A set of arguments to provide to the pre-commit hook identified by `id`. These arguments overwrite any existing arguments. | -| `additional_args` | A set of arguments to provide to the pre-commit hook identified by `id`. These arguments are appended after an existing arguments. | -| `exclude_file_patterns` | A set of file patterns to provide to pre-commit to ignore for the purposes of this hook. Use a typical file pattern you might find in a .gitignore file, such as `*.py` or `tests/`. Certain patterns you will have to wrap in double-quotes for the entry to be valid YAML. | ## Using Observability Platform to Show Secret Detection Statistics @@ -168,7 +144,7 @@ Should you need seCureLI to work with other platforms, please create a new issue - Once the above setup is complete, everytime seCureLI triggered, it should send a usage log to New Relic - In New Relic, you can create a dashboard of metric to see the number of times secret was caught using query such as -```commandline +```pre FROM Log Select sum(failure_count_details.detect_secrets) as 'Caught Secret Count' ``` diff --git a/secureli/actions/action.py b/secureli/actions/action.py index 8bed7733..2e4428dc 100644 --- a/secureli/actions/action.py +++ b/secureli/actions/action.py @@ -8,7 +8,6 @@ from secureli.abstractions.echo import EchoAbstraction, Color from secureli.abstractions.pre_commit import ( InstallFailedError, - PreCommitAbstraction, ) from secureli.repositories.secureli_config import ( SecureliConfig, @@ -17,10 +16,10 @@ ) from secureli.repositories.settings import SecureliRepository from secureli.services.language_analyzer import LanguageAnalyzerService, AnalyzeResult +from secureli.services.language_config import LanguageNotSupportedError from secureli.services.language_support import LanguageSupportService from secureli.services.scanner import ScannerService, ScanMode from secureli.services.updater import UpdaterService -from secureli.services.language_config import LanguageNotSupportedError class VerifyOutcome(str, Enum): @@ -102,26 +101,6 @@ def verify_install( if not config.languages or not config.version_installed: return self._install_secureli(folder_path, always_yes) else: - available_version = self.action_deps.language_support.version_for_language( - config.languages - ) - - # Check for a new version and prompt for upgrade if available - if available_version != config.version_installed: - return self._upgrade_secureli(config, available_version, always_yes) - - # Validates the current .pre-commit-config.yaml against the generated config - config_validation_result = ( - self.action_deps.language_support.validate_config( - languages=config.languages - ) - ) - - # If config mismatch between available version and current version prompt for upgrade - if not config_validation_result.successful: - self.action_deps.echo.print(config_validation_result.output) - return self._update_secureli(always_yes) - self.action_deps.echo.print( f"seCureLI is installed and up-to-date (languages = {config.languages})" ) @@ -130,50 +109,6 @@ def verify_install( config=config, ) - def _upgrade_secureli( - self, config: SecureliConfig, available_version: str, always_yes: bool - ) -> VerifyResult: - """ - Installs seCureLI into the given folder path and returns the new configuration - :param config: The existing configuration for seCureLI - :param available_version: The new version we're upgrading to - :param always_yes: Assume "Yes" to all prompts - :return: The new SecureliConfig after upgrade or None if upgrading did not complete - """ - self.action_deps.echo.print( - f"The config version installed is {config.version_installed}, but the latest is {available_version}" - ) - response = always_yes or self.action_deps.echo.confirm( - "Upgrade now?", - default_response=True, - ) - if not response: - self.action_deps.echo.warning("User canceled upgrade process") - return VerifyResult( - outcome=VerifyOutcome.UPGRADE_CANCELED, - config=config, - ) - - try: - metadata = self.action_deps.language_support.apply_support(config.languages) - - # Update config with new version installed and save it - config.version_installed = metadata.version - self.action_deps.secureli_config.save(config) - self.action_deps.echo.print("seCureLI has been upgraded successfully") - return VerifyResult( - outcome=VerifyOutcome.UPGRADE_SUCCEEDED, - config=config, - ) - except InstallFailedError: - self.action_deps.echo.error( - "seCureLI could not be upgraded due to an error" - ) - return VerifyResult( - outcome=VerifyOutcome.UPGRADE_FAILED, - config=config, - ) - def _install_secureli(self, folder_path: Path, always_yes: bool) -> VerifyResult: """ Installs seCureLI into the given folder path and returns the new configuration diff --git a/secureli/actions/scan.py b/secureli/actions/scan.py index 6b4cec7d..0405b203 100644 --- a/secureli/actions/scan.py +++ b/secureli/actions/scan.py @@ -1,24 +1,15 @@ +import json from pathlib import Path from typing import Optional -from secureli.utilities.usage_stats import post_log, convert_failures_to_failure_count from secureli.abstractions.echo import EchoAbstraction +from secureli.actions.action import VerifyOutcome, Action, ActionDependencies from secureli.services.logging import LoggingService, LogAction from secureli.services.scanner import ( ScanMode, ScannerService, - Failure, - OutputParseErrors, -) -from secureli.actions.action import VerifyOutcome, Action, ActionDependencies -from secureli.repositories.settings import ( - SecureliRepository, - SecureliFile, - PreCommitSettings, - PreCommitRepo, - PreCommitHook, ) -import json +from secureli.utilities.usage_stats import post_log, convert_failures_to_failure_count class ScanAction(Action): @@ -36,13 +27,11 @@ def __init__( echo: EchoAbstraction, logging: LoggingService, scanner: ScannerService, - # settings_repository: SecureliRepository, ): super().__init__(action_deps) self.scanner = scanner self.echo = echo self.logging = logging - # self.settings = settings_repository def scan_repo( self, @@ -80,9 +69,6 @@ def scan_repo( scan_result.failures ) - if failure_count > 0: - self._process_failures(scan_result.failures, always_yes=always_yes) - if not scan_result.successful: log_data = self.logging.failure( LogAction.scan, @@ -97,180 +83,3 @@ def scan_repo( log_data = self.logging.success(LogAction.scan) post_log(log_data.json(exclude_none=True)) - - def _process_failures( - self, - failures: list[Failure], - always_yes: bool, - ): - """ - Processes any failures found during the scan. - :param failures: List of Failure objects representing linter failures - :param always_yes: Assume "Yes" to all prompts - """ - settings = self.action_deps.settings.load() - - ignore_fail_prompt = "Failures detected during scan.\n" - ignore_fail_prompt += "Add an ignore rule?" - - # Ask if the user wants to ignore a failure - if always_yes: - always_yes_warning = "Hook failures were detected but the scan was initiated with the 'yes' flag.\n" - always_yes_warning += "seCureLI cannot automatically add ignore rules with the 'yes' flag enabled.\n" - always_yes_warning += "Re-run your scan without the 'yes' flag to add an ignore rule for one of the\n" - always_yes_warning += "detected failures." - - self.echo.print(always_yes_warning) - elif self.echo.confirm(ignore_fail_prompt, default_response=False): - # verify pre_commit exists in settings file. - if not settings.pre_commit: - settings.pre_commit = PreCommitSettings() - - for failure in failures: - add_ignore_for_id = self.echo.confirm( - "\nWould you like to add an ignore for the {} failure on {}?".format( - failure.id, failure.file - ) - ) - if failure.repo == OutputParseErrors.REPO_NOT_FOUND: - self._handle_repo_not_found(failure) - elif always_yes or add_ignore_for_id: - settings = self._add_ignore_for_failure( - failure=failure, always_yes=always_yes, settings_file=settings - ) - - self.action_deps.settings.save(settings=settings) - - def _add_ignore_for_failure( - self, - failure: Failure, - always_yes: bool, - settings_file: SecureliFile, - ): - """ - Processes an individual failure and adds an ignore rule for either the entire - hook or a particular file. - :param failure: Failure object representing a rule failure during a scan - :param always_yes: Assume "Yes" to all prompts - :param settings_file: SecureliFile representing the contents of the .secureli.yaml file - """ - ignore_repo_prompt = "You can add an ignore rule for just this file, or you can add an ignore rule for all files.\n" - ignore_repo_prompt += ( - "Would you like to ignore this failure for all files?".format(failure.id) - ) - ignore_file_prompt = ( - "\nWould you like to ignore this failure for just the {} file?".format( - failure.file - ) - ) - - self.echo.print("\nAdding an ignore rule for: {}\n".format(failure.id)) - - if always_yes or self.echo.confirm( - message=ignore_repo_prompt, default_response=False - ): - # ignore for all files - self.echo.print("Adding an ignore for all files.") - modified_settings = self._ignore_all_files( - failure=failure, settings_file=settings_file - ) - else: - if always_yes or self.echo.confirm(ignore_file_prompt, False): - self.echo.print("Adding an ignore for {}".format(failure.file)) - modified_settings = self._ignore_one_file( - failure=failure, settings_file=settings_file - ) - else: - self.echo.print( - "\nSkipping {} failure on {}".format(failure.id, failure.file) - ) - modified_settings = settings_file - - return modified_settings - - def _handle_repo_not_found(self, failure: Failure): - """ - Handles a REPO_NOT_FOUND error - :param failure: A Failure object representing the scan failure with a missing repo url - """ - id = failure.id - self.echo.print( - "Unable to add an ignore for {}, seCureLI was unable to identify the repo it belongs to.".format( - failure.id - ) - ) - self.echo.print("Skipping {}".format(id)) - - def _ignore_all_files(self, failure: Failure, settings_file: SecureliFile): - """ - Supresses a hook for all files in this repo - :param failure: Failure object representing the failed hook - :param settings_file: SecureliFile representing the contents of the .secureli.yaml file - :return: Returns the settings file after modifications - """ - pre_commit_settings = settings_file.pre_commit - repos = pre_commit_settings.repos - repo_settings_index = next( - (index for (index, repo) in enumerate(repos) if repo.url == failure.repo), - None, - ) - - if repo_settings_index is not None: - repo_settings = pre_commit_settings.repos[repo_settings_index] - if failure.id not in repo_settings.suppressed_hook_ids: - repo_settings.suppressed_hook_ids.append(failure.id) - else: - repo_settings = PreCommitRepo( - url=failure.repo, suppressed_hook_ids=[failure.id] - ) - repos.append(repo_settings) - - self.echo.print( - "Added {} to the suppressed_hooks_ids list for the {} repo".format( - failure.id, failure.repo - ) - ) - - return settings_file - - def _ignore_one_file(self, failure: Failure, settings_file: SecureliFile): - """ - Adds the failed file to the file exemptions list for the failed hook - :param failure: Failure object representing the failed hook - :param settings_file: SecureliFile representing the contents of the .secureli.yaml file - """ - pre_commit_settings = settings_file.pre_commit - repos = pre_commit_settings.repos - repo_settings_index = next( - (index for (index, repo) in enumerate(repos) if repo.url == failure.repo), - None, - ) - - if repo_settings_index is not None: - repo_settings = pre_commit_settings.repos[repo_settings_index] - else: - repo_settings = PreCommitRepo(url=failure.repo) - repos.append(repo_settings) - - hooks = repo_settings.hooks - hook_settings_index = next( - (index for (index, hook) in enumerate(hooks) if hook.id == failure.id), - None, - ) - - if hook_settings_index is not None: - hook_settings = hooks[hook_settings_index] - if failure.file not in hook_settings.exclude_file_patterns: - hook_settings.exclude_file_patterns.append(failure.file) - else: - self.echo.print( - "An ignore rule is already present for the {} file".format( - failure.file - ) - ) - else: - hook_settings = PreCommitHook(id=failure.id) - hook_settings.exclude_file_patterns.append(failure.file) - repo_settings.hooks.append(hook_settings) - - return settings_file diff --git a/secureli/container.py b/secureli/container.py index e2a3c613..f856b626 100644 --- a/secureli/container.py +++ b/secureli/container.py @@ -97,7 +97,6 @@ class Container(containers.DeclarativeContainer): language_config_service = providers.Factory( LanguageConfigService, data_loader=read_resource, - pre_commit_settings=config.pre_commit, command_timeout_seconds=config.language_support.command_timeout_seconds, ignored_file_patterns=secureli_ignored_file_patterns, ) diff --git a/secureli/repositories/settings.py b/secureli/repositories/settings.py index 0bfe5317..905d915a 100644 --- a/secureli/repositories/settings.py +++ b/secureli/repositories/settings.py @@ -1,11 +1,10 @@ -from pathlib import Path -from typing import Optional, Literal from enum import Enum -import yaml +from pathlib import Path +from typing import Optional +import yaml from pydantic import BaseModel, BaseSettings, Field - default_ignored_extensions = [ # Images ".png", @@ -130,7 +129,6 @@ class SecureliFile(BaseModel): repo_files: Optional[RepoFilesSettings] echo: Optional[EchoSettings] language_support: Optional[LanguageSupportSettings] = Field(default=None) - pre_commit: Optional[PreCommitSettings] = Field(default=None) class SecureliRepository: diff --git a/secureli/services/language_config.py b/secureli/services/language_config.py index 0c3f8fc3..1b47c843 100644 --- a/secureli/services/language_config.py +++ b/secureli/services/language_config.py @@ -1,14 +1,12 @@ from pathlib import Path -from typing import Callable, Optional, Any +from typing import Callable, Any -import pathspec import pydantic import yaml -from secureli.repositories.settings import PreCommitSettings, PreCommitRepo -from secureli.utilities.patterns import combine_patterns from secureli.resources.slugify import slugify from secureli.utilities.hash import hash_config +from secureli.utilities.patterns import combine_patterns class LanguageNotSupportedError(Exception): @@ -41,16 +39,10 @@ def __init__( command_timeout_seconds: int, data_loader: Callable[[str], str], ignored_file_patterns: list[str], - pre_commit_settings: dict[str:Any], ): self.command_timeout_seconds = command_timeout_seconds self.data_loader = data_loader self.ignored_file_patterns = ignored_file_patterns - self.pre_commit_settings = ( - PreCommitSettings.parse_obj(pre_commit_settings) - if pre_commit_settings - else PreCommitSettings() - ) def get_language_config(self, language: str) -> LanguagePreCommitResult: """ @@ -91,152 +83,8 @@ def _calculate_combined_configuration(self, language: str) -> dict: if self.ignored_file_patterns: config["exclude"] = combine_patterns(self.ignored_file_patterns) - # Combine our .secureli.yaml mutations into the configuration - self._apply_pre_commit_settings(config) - - return config - - def _find_matching_hook( - self, - hook_id: str, - pre_commit_repo_hooks: dict, - ) -> Optional[dict]: - """ - Find a hook from the pre-commit configuration matching the given hook ID - :param hook_id: The hook ID to find within the repo - :param pre_commit_repo_hooks: The dictionary representing the repo configuration - :return: The dictionary representing the hook configuration within the given - repository, or None if the hook was not present - """ - matching_pre_commit_hooks = [ - pre_commit_hook - for pre_commit_hook in pre_commit_repo_hooks - if pre_commit_hook["id"] == hook_id - ] - return matching_pre_commit_hooks[0] if matching_pre_commit_hooks else None - - def _apply_pre_commit_settings(self, config: dict) -> dict: - """ - Check our pre-commit settings derived from the .secureli.yaml file and merge them - into our pre-commit configuration, if any - """ - if "repos" not in config: - return config - - for pre_commit_repo in config["repos"]: - repo_url = pre_commit_repo["repo"] - - # If we're suppressing an entire repo, remove it and ignore all other mutations - if repo_url in self.pre_commit_settings.suppressed_repos: - config["repos"].remove(pre_commit_repo) - continue - - matching_settings_repo = self._find_matching_repo_settings(repo_url) - - if not matching_settings_repo: - continue - - # The hook may have been suppressed within the repo, via .secureli.yaml. Remove it if so. - self._remove_suppressed_hooks( - config, - pre_commit_repo, - matching_settings_repo, - ) - - pre_commit_repo_hooks = pre_commit_repo["hooks"] - - for hook_settings in matching_settings_repo.hooks or []: - # Find the hook from the configuration (it may have just been removed) - matching_hook = self._find_matching_hook( - hook_settings.id, - pre_commit_repo_hooks, - ) - - if not matching_hook: - continue - - # We found a hook in our settings that matches a hook in our config. Update - # the config with our settings. - - # First we've got arguments to overwrite hook configuration if we supplied overrides - # Note: this discards any original arguments if present. - self._override_arguments_if_any( - matching_hook, - hook_settings.arguments, - ) - - # Second we've got additional arguments to set or append into the hook. This is a good way - # to go with whatever arguments might be present, but also add additional ones. It would be - # strange if we specified both `arguments` and `additional_arguments`, but you do you. - self._apply_additional_args( - matching_hook, - hook_settings.additional_args, - ) - - # We can also specify file patterns to ignore. Do our usual routine to turn this - # into a combined pattern we can provide to pre-commit - self._apply_file_exclusions( - matching_hook, - hook_settings.exclude_file_patterns, - ) - - # Room to keep supporting more secureli-configurable pre-commit stuff, probably coming soon. - return config - def _override_arguments_if_any( - self, - pre_commit_hook: dict, - arguments: Optional[list[str]], - ): - """ - Override the hook configuration dictionary with the arguments provided, unless the arguments was None. - If the arguments were empty, any existing arguments will be removed. If the arguments were None, the - existing argument configuration will be left unchanged. - """ - # We may need to wipe out arguments, so we can't use the typical `if not arguments:` - if arguments is None: - return - - pre_commit_hook["args"] = arguments - - def _apply_additional_args( - self, - pre_commit_hook: dict, - additional_args: list[str], - ): - """ - Append the provided additional arguments into the hook configuration, always preserving - any existing arguments - """ - if not additional_args: - return - - if "args" not in pre_commit_hook: - pre_commit_hook["args"] = [] - - for additional_arg in additional_args: - pre_commit_hook["args"].append(additional_arg) - - def _apply_file_exclusions( - self, matching_hook: dict, exclude_file_patterns: Optional[list[str]] - ): - """ - Calculate the single regex for the given file exclusion patterns and add to the hook config. - """ - if not exclude_file_patterns: - return - - pathspec_lines = pathspec.PathSpec.from_lines( - pathspec.patterns.GitWildMatchPattern, exclude_file_patterns - ) - raw_patterns = [ - pathspec_pattern.regex.pattern - for pathspec_pattern in pathspec_lines.patterns - if pathspec_pattern.include - ] - matching_hook["exclude"] = combine_patterns(raw_patterns) - def _calculate_combined_configuration_data(self, language: str) -> str: """ Combine elements of our configuration for the specified language along with @@ -248,41 +96,6 @@ def _calculate_combined_configuration_data(self, language: str) -> str: config = self._calculate_combined_configuration(language) return yaml.dump(config) - def _find_matching_repo_settings(self, repo_url: str) -> Optional[PreCommitRepo]: - """ - Find a repo from our settings that matches the given URL - :param repo_url: The URL of the repo - :return: A PreCommitRepo settings object, as managed from .secureli.yaml, or None - if a match wasn't found. - """ - matching_settings_repos = [ - repo - for repo in self.pre_commit_settings.repos - if repo.url.lower() == repo_url.lower() - ] - return matching_settings_repos[0] if matching_settings_repos else None - - def _remove_suppressed_hooks( - self, config: dict, pre_commit_repo: dict, matching_settings_repo: PreCommitRepo - ): - """ - Remove any suppressed hook from the provided repo configuration dictionary completely. - If the last hook in a repo is removed by this process, the repo itself is removed from - the configuration completely as well. - """ - for hook_id in matching_settings_repo.suppressed_hook_ids: - # This hook is configured by .secureli.yaml to be suppressed, so remove it (and by 'it' - # I mean any that match the hook's ID, because it's an array) - matching_repo_hooks = [ - x for x in pre_commit_repo.get("hooks") if x["id"] == hook_id - ] - for hook in matching_repo_hooks: - pre_commit_repo["hooks"].remove(hook) - - # If we removed the last hook from the repo, there's no point to the repo anymore, so remove it. - if not pre_commit_repo["hooks"] and pre_commit_repo in config["repos"]: - config["repos"].remove(pre_commit_repo) - def _load_linter_config_file(self, language: str) -> LoadLinterConfigsResult: """ Load any config files for given language if they exist. diff --git a/secureli/services/language_support.py b/secureli/services/language_support.py index 6b75de28..6fd034e8 100644 --- a/secureli/services/language_support.py +++ b/secureli/services/language_support.py @@ -1,14 +1,14 @@ +from pathlib import Path from typing import Callable, Optional, Any import pydantic import yaml -from pathlib import Path from secureli.abstractions.pre_commit import PreCommitAbstraction +from secureli.resources.slugify import slugify from secureli.services.git_ignore import GitIgnoreService from secureli.services.language_config import LanguageConfigService from secureli.utilities.hash import hash_config -from secureli.resources.slugify import slugify supported_languages = [ "C#", @@ -100,16 +100,6 @@ def __init__( self.language_config = language_config self.data_loader = data_loader - def version_for_language(self, languages: list[str]) -> str: - """ - May eventually grow to become a combination of pre-commit hook and other elements - :param languages: List of languages to determine the version of the current config - :raises LanguageNotSupportedError if support for the language is not provided - :return: The version of the current config for the provided language available for install - """ - # For now, just a passthrough to pre-commit hook abstraction - return self._build_pre_commit_config(languages).version - def apply_support(self, languages: list[str]) -> LanguageMetadata: """ Applies Secure Build support for the provided language @@ -181,33 +171,6 @@ def secret_detection_hook_id(self, languages: list[str]) -> Optional[str]: return None - def validate_config(self, languages: list[str]) -> ValidateConfigResult: - """ - Validates that the current configuration matches the expected configuration generated - by secureli. - :param language: List of languages to validate against - :return: Returns a boolean indicating whether the configs match - """ - current_config = yaml.dump(self.get_current_configuration()) - generated_config = self._build_pre_commit_config(languages) - current_hash = self.get_current_config_hash() - expected_hash = generated_config.version - output = "" - - config_matches = current_hash == expected_hash - - if not config_matches: - output += "seCureLI has detected that the .pre-commit-config.yaml file does not match the expected configuration.\n" - output += "This often occurs when the .pre-commit-config.yaml file has been modified directly.\n" - output += "All changes to seCureLI's configuration should be performed through the .secureli.yaml file.\n" - output += "\n" - output += self._compare_repo_versions( - current_config=yaml.safe_load(current_config), - expected_config=generated_config.config_data, - ) - - return ValidateConfigResult(successful=config_matches, output=output) - def get_configuration(self, languages: list[str]) -> HookConfiguration: """ Creates a basic, serializable configuration out of the combined specified language config @@ -226,36 +189,11 @@ def create_repo(raw_repo: dict) -> Repo: repos = [create_repo(raw_repo) for raw_repo in config.get("repos", [])] return HookConfiguration(repos=repos) - def get_current_configuration(self): - """ - Returns the contents of the .pre-commit-config.yaml file. Note that this should be used to - see the current state and not be used for any desired state actions. - :return: Dictionary containing the contents of the .pre-commit-config.yaml file - """ - path_to_pre_commit_file = Path(".pre-commit-config.yaml") - - with open(path_to_pre_commit_file, "r") as f: - data = yaml.safe_load(f) - return data - - def get_current_config_hash(self) -> str: - """ - Returns a hash of the current .pre-commit-config.yaml file. This hash is generated in the - same way that we generate the version hash for the secureli config file so should be valid - for comparison. Note this is the hash of the config file as it currently exists and not - the hash of the combined config. - :return: Returns a hash derived from the - """ - config_data = yaml.dump(self.get_current_configuration()) - config_hash = hash_config(config_data) - - return config_hash - def _build_pre_commit_config(self, languages: list[str]) -> BuildConfigResult: """ Builds the final .pre-commit-config.yaml from all supported repo languages. Also returns any and all linter configuration data. - :param langauges: list of languages to get calculated configuration for. + :param languages: list of languages to get calculated configuration for. :return: BuildConfigResult """ config_data = [] @@ -289,157 +227,14 @@ def _build_pre_commit_config(self, languages: list[str]) -> BuildConfigResult: linter_configs=linter_configs, ) - def _get_list_of_repo_urls(self, repo_list: list[dict]) -> list[str]: - """ - Parses a list containing repo dictionaries and returns a list of repo urls - :param repo_list: List of dictionaries containing repo configurations - :return: A list of repo urls. - """ - urls = [] - - for repo in repo_list: - urls.append(repo["repo"]) - - return urls - - def _get_dict_with_repo_revs(self, repo_list: list[dict]) -> dict: - """ - Parses a list containing repo dictionaries and returns a dictionary which - contains the repo name as the key and rev as the value. - :param repo_list: List of dictionaries containing repo configurations - :return: A dict with the repo urls as the key and the repo rev as the value. - """ - repos_dict = {} - - for repo in repo_list: - url = repo["repo"] - rev = repo["rev"] - repos_dict[url] = rev - - return repos_dict - - def _process_mismatched_repo_versions( - self, current_repos: list[dict], expected_repos: list[dict] - ): - """ - Processes the list of repos from the .pre-commit-config.yaml and the expected (generated) config - and returns a output as a string which lists any version mismatches detected. - :param current_repos: List of dictionaries containing repo configurations from the .pre-commit-config.yaml - file - :param expected_repos: List of dictionaries containing repo configurations from the expected (generated) - config - :return: Returns a string of output representing the version mismatches that were detected - """ - current_repos_dict = self._get_dict_with_repo_revs(repo_list=current_repos) - expected_repos_dict = self._get_dict_with_repo_revs(repo_list=expected_repos) - output = "" - - for repo in expected_repos_dict: - expected_rev = expected_repos_dict.get(repo) - current_rev = current_repos_dict.get(repo) - if expected_rev != current_rev: - output += ( - "Expected {} to be rev {} but it is configured to rev {}\n".format( - repo, expected_rev, current_rev - ) - ) - - return output - - def _get_mismatched_repos(self, current_repos: list, expected_repos: list): - """ - Compares the list of repos in the current config against the list of repos - in the expected (generated) config and returns an object with a list of missing - repos and a list of unexpected repos. - """ - current_repos_set = set(current_repos) - expected_repos_set = set(expected_repos) - unexpected_repos = [ - repo for repo in current_repos if repo not in expected_repos_set - ] - missing_repos = [ - repo for repo in expected_repos if repo not in current_repos_set - ] - - return UnexpectedReposResult( - missing_repos=missing_repos, unexpected_repos=unexpected_repos - ) - - def _process_repo_list_length_mismatch( - self, current_repos: list[str], expected_repos: list[str] - ): - """ - Processes the repo lists for the current config (.pre-commit-config.yaml) and the expected - (generated) config and generates text output indicating which repos are unexpected and - which repos are missing. - :param current_repos: List of repo names that are in the .pre-commit-config.yaml file - :param expected_repos: List of repo names from the expected (generated) config - :return: Returns output in string format with the results of the comparison - """ - output = "" - - mismatch_results = self._get_mismatched_repos( - current_repos=current_repos, - expected_repos=expected_repos, - ) - unexpected_repos = mismatch_results.unexpected_repos - missing_repos = mismatch_results.missing_repos - - if len(unexpected_repos) > 0: - output += "Found unexpected repos in .pre-commit-config.yaml:\n" - for repo in unexpected_repos: - output += "- {}\n".format(repo) - - output += "\n" - - if len(missing_repos) > 0: - output += ( - "Some expected repos were misssing from .pre-commit-config.yaml:\n" - ) - for repo in missing_repos: - output += "- {}\n".format(repo) - - output += "\n" - - return output - - def _compare_repo_versions(self, current_config: dict, expected_config: dict): - """ - Compares the current config and expected (generated) config and detemines if there - are version mismatches for the hooks. - :param current_config: The current configuration as a dict - :param expected_config: The expected (generated) configuration as a dict - :return: Returns a string containing the differences between the two configs. - """ - current_config_repos = current_config.get("repos", []) - expected_config_repos = expected_config.get("repos", []) - output = "Comparing current .pre-commit-config.yaml to expected configuration\n" - - length_of_repos_lists_match = len(current_config_repos) == len( - expected_config_repos - ) - - if not length_of_repos_lists_match: - output += self._process_repo_list_length_mismatch( - current_repos=self._get_list_of_repo_urls(current_config_repos), - expected_repos=self._get_list_of_repo_urls(expected_config_repos), - ) - - output += self._process_mismatched_repo_versions( - current_repos=current_config_repos, - expected_repos=expected_config_repos, - ) - - return output - + @staticmethod def _write_pre_commit_configs( - self, all_linter_configs: list[LinterConfig] + all_linter_configs: list[LinterConfig], ) -> LanguageLinterWriteResult: """ Install any config files for given language to support any pre-commit commands. i.e. Javascript ESLint requires a .eslintrc file to sufficiently use plugins and allow for further customization for repo's flavor of Javascript - :param language: repo language :return: LanguageLinterWriteResult """ diff --git a/secureli/settings.py b/secureli/settings.py index d9f7a85b..8e6adfe6 100644 --- a/secureli/settings.py +++ b/secureli/settings.py @@ -1,5 +1,5 @@ from pathlib import Path -from typing import Any, Optional +from typing import Any import pydantic import yaml @@ -8,7 +8,6 @@ RepoFilesSettings, EchoSettings, LanguageSupportSettings, - PreCommitSettings, ) @@ -41,7 +40,6 @@ class Settings(pydantic.BaseSettings): repo_files: RepoFilesSettings = RepoFilesSettings() echo: EchoSettings = EchoSettings() language_support: LanguageSupportSettings = LanguageSupportSettings() - pre_commit: PreCommitSettings = PreCommitSettings() class Config: env_file_encoding = "utf-8" diff --git a/tests/abstractions/test_pre_commit.py b/tests/abstractions/test_pre_commit.py index 6841e302..f1edac89 100644 --- a/tests/abstractions/test_pre_commit.py +++ b/tests/abstractions/test_pre_commit.py @@ -9,29 +9,6 @@ from secureli.abstractions.pre_commit import ( PreCommitAbstraction, ) -from secureli.repositories.settings import ( - PreCommitSettings, - PreCommitRepo, - PreCommitHook, -) - - -@pytest.fixture() -def settings_dict() -> dict: - return PreCommitSettings( - repos=[ - PreCommitRepo( - url="http://example-repo.com/", - hooks=[ - PreCommitHook( - id="hook-id", - arguments=None, - additional_args=None, - ) - ], - ) - ] - ).dict() @pytest.fixture() diff --git a/tests/actions/test_action.py b/tests/actions/test_action.py index e3b300dd..d3f57cf6 100644 --- a/tests/actions/test_action.py +++ b/tests/actions/test_action.py @@ -3,11 +3,10 @@ import pytest -from secureli.abstractions.pre_commit import InstallFailedError +from secureli.actions.action import Action, ActionDependencies, VerifyOutcome from secureli.repositories.secureli_config import SecureliConfig, VerifyConfigOutcome from secureli.services.language_analyzer import AnalyzeResult, SkippedFile -from secureli.actions.action import Action, ActionDependencies, VerifyOutcome -from secureli.services.language_support import LanguageMetadata, ValidateConfigResult +from secureli.services.language_support import LanguageMetadata from secureli.services.scanner import ScanResult, Failure from secureli.services.updater import UpdateResult @@ -197,23 +196,6 @@ def test_that_initialize_repo_selects_previously_selected_language( ) -def test_that_initialize_repo_prompts_to_upgrade_when_out_of_sync( - action: Action, - mock_secureli_config: MagicMock, - mock_language_support: MagicMock, - mock_echo: MagicMock, -): - mock_secureli_config.load.return_value = SecureliConfig( - languages=["PreviousLang"], version_installed="abc123" - ) - mock_language_support.version_for_language.return_value = "xyz987" - mock_echo.confirm.return_value = False - - action.verify_install(test_folder_path, reset=False, always_yes=False) - - mock_echo.warning.assert_called_with("User canceled upgrade process") - - def test_that_initialize_repo_prompts_to_upgrade_config_if_old_schema( action: Action, mock_secureli_config: MagicMock, @@ -253,39 +235,6 @@ def test_that_initialize_repo_updates_repo_config_if_old_schema( assert result.outcome == VerifyOutcome.UP_TO_DATE -def test_that_initialize_repo_auto_upgrades_when_out_of_sync( - action: Action, - mock_secureli_config: MagicMock, - mock_language_support: MagicMock, - mock_echo: MagicMock, -): - mock_secureli_config.load.return_value = SecureliConfig( - languages=["PreviousLang"], version_installed="abc123" - ) - mock_language_support.version_for_language.return_value = "xyz987" - - action.verify_install(test_folder_path, reset=False, always_yes=True) - - mock_echo.print.assert_called_with("seCureLI has been upgraded successfully") - - -def test_that_initialize_repo_reports_errors_when_upgrade_fails( - action: Action, - mock_secureli_config: MagicMock, - mock_language_support: MagicMock, - mock_echo: MagicMock, -): - mock_secureli_config.load.return_value = SecureliConfig( - languages=["PreviousLang"], version_installed="abc123" - ) - mock_language_support.version_for_language.return_value = "xyz987" - mock_language_support.apply_support.side_effect = InstallFailedError - - action.verify_install(test_folder_path, reset=False, always_yes=True) - - mock_echo.error.assert_called_with("seCureLI could not be upgraded due to an error") - - def test_that_initialize_repo_reports_errors_when_schema_upgdate_fails( action: Action, mock_secureli_config: MagicMock, @@ -316,47 +265,35 @@ def test_that_initialize_repo_is_aborted_by_the_user_if_the_process_is_canceled( mock_echo.error.assert_called_with("User canceled install process") -def test_that_verify_install_updates_if_config_validation_fails( +def test_that_update_secureli_handles_declined_update( action: Action, - mock_language_support: MagicMock, - mock_updater: MagicMock, - mock_secureli_config: MagicMock, + mock_echo: MagicMock, ): - mock_language_support.validate_config.return_value = ValidateConfigResult( - successful=False, output="Configs don't match" - ) - mock_language_support.version_for_language.return_value = "abc123" - mock_secureli_config.load.return_value = SecureliConfig( - languages=["PreviousLang"], version_installed="abc123" - ) - mock_updater.update.return_value = UpdateResult( - successful=True, output="Some output" - ) - - verify_result = action.verify_install( - test_folder_path, reset=False, always_yes=True - ) + mock_echo.confirm.return_value = False + update_result = action._update_secureli(always_yes=False) - assert verify_result.outcome == "update-succeeded" + assert update_result.outcome == VerifyOutcome.UPDATE_CANCELED -def test_that_update_secureli_handles_declined_update( +def test_that_update_secureli_handles_failed_update( action: Action, - mock_echo: MagicMock, + mock_updater: MagicMock, ): - mock_echo.confirm.return_value = False + mock_updater.update.return_value = UpdateResult( + successful=False, outcome=VerifyOutcome.UPDATE_FAILED + ) update_result = action._update_secureli(always_yes=False) - assert update_result.outcome == "update-canceled" + assert update_result.outcome == VerifyOutcome.UPDATE_FAILED -def test_that_update_secureli_handles_failed_update( +def test_that_update_secureli_handles_successful_update( action: Action, mock_updater: MagicMock, ): mock_updater.update.return_value = UpdateResult( - successful=False, outcome="update failed" + successful=True, outcome=VerifyOutcome.UPDATE_SUCCEEDED ) update_result = action._update_secureli(always_yes=False) - assert update_result.outcome == "update-failed" + assert update_result.outcome == VerifyOutcome.UPDATE_SUCCEEDED diff --git a/tests/actions/test_scan_action.py b/tests/actions/test_scan_action.py index b50e819f..954b1131 100644 --- a/tests/actions/test_scan_action.py +++ b/tests/actions/test_scan_action.py @@ -1,8 +1,8 @@ +import os from pathlib import Path from unittest import mock -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock -import os import pytest from secureli.actions.action import ActionDependencies @@ -10,15 +10,10 @@ from secureli.repositories.secureli_config import SecureliConfig from secureli.repositories.settings import ( SecureliFile, - PreCommitSettings, - PreCommitRepo, - PreCommitHook, EchoSettings, EchoLevel, - LanguageSupportSettings, - RepoFilesSettings, ) -from secureli.services.scanner import ScanMode, ScanResult, Failure, OutputParseErrors +from secureli.services.scanner import ScanMode, ScanResult, Failure test_folder_path = Path("does-not-matter") @@ -47,17 +42,9 @@ def mock_default_settings(mock_settings_repository: MagicMock) -> MagicMock: @pytest.fixture() def mock_default_settings_populated(mock_settings_repository: MagicMock) -> MagicMock: - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) mock_echo_settings = EchoSettings(EchoLevel.info) - mock_pre_commit_hook_settings = PreCommitHook(id=mock_failure.id) - mock_pre_commit_repo_settings = PreCommitRepo( - url=mock_failure.repo, hooks=[mock_pre_commit_hook_settings] - ) - mock_pre_commit_settings = PreCommitSettings(repos=[mock_pre_commit_repo_settings]) mock_settings_file = SecureliFile( - echo=mock_echo_settings, pre_commit=mock_pre_commit_settings + echo=mock_echo_settings, ) mock_settings_repository.load.return_value = mock_settings_file @@ -68,21 +55,9 @@ def mock_default_settings_populated(mock_settings_repository: MagicMock) -> Magi def mock_default_settings_ignore_already_exists( mock_settings_repository: MagicMock, ) -> MagicMock: - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) mock_echo_settings = EchoSettings(EchoLevel.info) - mock_pre_commit_hook_settings = PreCommitHook( - id=mock_failure.id, exclude_file_patterns=[mock_failure.file] - ) - mock_pre_commit_repo_settings = PreCommitRepo( - url=mock_failure.repo, - hooks=[mock_pre_commit_hook_settings], - suppressed_hook_ids=[mock_failure.id], - ) - mock_pre_commit_settings = PreCommitSettings(repos=[mock_pre_commit_repo_settings]) mock_settings_file = SecureliFile( - echo=mock_echo_settings, pre_commit=mock_pre_commit_settings + echo=mock_echo_settings, ) mock_settings_repository.load.return_value = mock_settings_file @@ -200,265 +175,6 @@ def test_that_scan_repo_does_not_scan_if_not_installed( mock_scanner.scan_repo.assert_not_called() -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_handles_declining_to_add_ignore_for_failures( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failures = [ - Failure(repo="some-repo", id="some-hook-id", file="some-failed-file.py") - ] - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=mock_failures - ) - mock_echo.confirm.return_value = False - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - mock_settings_repository.load.assert_called() - mock_settings_repository.save.assert_not_called() - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_adds_ignore_for_all_files_when_prompted( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_suppressed_id = saved_settings.pre_commit.repos[0].suppressed_hook_ids[0] - expected_suppressed_id = mock_failure.id - - assert saved_suppressed_id is expected_suppressed_id - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_adds_ignore_for_all_files_when_settings_exist_when_prompted( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings_populated: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_suppressed_id = saved_settings.pre_commit.repos[0].suppressed_hook_ids[0] - expected_suppressed_id = mock_failure.id - - assert saved_suppressed_id is expected_suppressed_id - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_skips_ignore_for_all_files_when_ignore_already_exists( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings_ignore_already_exists: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_suppressed_id = saved_settings.pre_commit.repos[0].suppressed_hook_ids[0] - expected_suppressed_id = mock_failure.id - - assert saved_suppressed_id is expected_suppressed_id - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_adds_ignore_for_one_file_when_prompted( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, False, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_excluded_file = ( - saved_settings.pre_commit.repos[0].hooks[0].exclude_file_patterns[0] - ) - expected_excluded_file = mock_failure.file - - assert saved_excluded_file is expected_excluded_file - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_adds_ignore_for_one_file_when_settings_exist_when_prompted( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings_populated: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, False, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_excluded_file = ( - saved_settings.pre_commit.repos[0].hooks[0].exclude_file_patterns[0] - ) - expected_excluded_file = mock_failure.file - - assert saved_excluded_file is expected_excluded_file - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_skips_ignore_for_one_file_when_ignore_already_present( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings_ignore_already_exists: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, False, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - saved_excluded_file = ( - saved_settings.pre_commit.repos[0].hooks[0].exclude_file_patterns[0] - ) - expected_excluded_file = mock_failure.file - - assert saved_excluded_file is expected_excluded_file - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_handles_missing_repo_while_adding_ignore_rule( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings_populated: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo=OutputParseErrors.REPO_NOT_FOUND, - id="some-hook-id", - file="some-failed-file.py", - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - mock_echo.print.assert_any_call( - "Unable to add an ignore for some-hook-id, seCureLI was unable to identify the repo it belongs to." - ) - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_does_not_add_ignore_if_both_ignore_types_declined( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, True, False, False] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - - assert saved_settings is mock_settings_repository.load.return_value - - -@mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) -def test_that_scan_repo_does_not_add_ignore_if_all_failures_declined( - scan_action: ScanAction, - mock_scanner: MagicMock, - mock_echo: MagicMock, - mock_settings_repository: MagicMock, - mock_default_settings: MagicMock, - mock_pass_install_verification: MagicMock, -): - mock_failure = Failure( - repo="some-repo", id="some-hook-id", file="some-failed-file.py" - ) - mock_scanner.scan_repo.return_value = ScanResult( - successful=True, output="some-output", failures=[mock_failure] - ) - mock_echo.confirm.side_effect = [True, False] - - scan_action.scan_repo(test_folder_path, ScanMode.STAGED_ONLY, False) - - saved_settings = mock_settings_repository.save.call_args.kwargs["settings"] - - assert saved_settings is mock_settings_repository.load.return_value - - @mock.patch.dict(os.environ, {"API_KEY": "", "API_ENDPOINT": ""}, clear=True) def test_that_scan_repo_does_not_add_ignore_if_always_yes_is_true( scan_action: ScanAction, diff --git a/tests/services/test_language_config.py b/tests/services/test_language_config.py index 3c78b6b6..0538ae99 100644 --- a/tests/services/test_language_config.py +++ b/tests/services/test_language_config.py @@ -1,18 +1,12 @@ -import pytest from unittest.mock import MagicMock +import pytest from secureli.services.language_config import ( LanguageConfigService, LanguageNotSupportedError, ) -from secureli.repositories.settings import ( - PreCommitSettings, - PreCommitRepo, - PreCommitHook, -) - @pytest.fixture() def mock_data_loader() -> MagicMock: @@ -21,34 +15,14 @@ def mock_data_loader() -> MagicMock: return mock_data_loader -@pytest.fixture() -def settings_dict() -> dict: - return PreCommitSettings( - repos=[ - PreCommitRepo( - url="http://example-repo.com/", - hooks=[ - PreCommitHook( - id="hook-id", - arguments=None, - additional_args=None, - ) - ], - ) - ] - ).dict() - - @pytest.fixture() def language_config_service( mock_data_loader: MagicMock, - settings_dict: dict, ) -> LanguageConfigService: return LanguageConfigService( command_timeout_seconds=300, data_loader=mock_data_loader, ignored_file_patterns=[], - pre_commit_settings=settings_dict, ) @@ -104,44 +78,6 @@ def test_that_language_config_service_templates_are_loaded_without_exclude( assert "exclude:" not in result.config_data -def test_that_language_config_service_overrides_arguments_in_a_security_hook( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, - settings_dict: dict, - mock_open: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://sample-repo.com/baddie-finder - hooks: - - id: baddie-finder-hook - args: - - orig_arg - """ - - language_config_service.pre_commit_settings.repos[ - 0 - ].url = "http://sample-repo.com/baddie-finder" - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].id = "baddie-finder-hook" - language_config_service.pre_commit_settings.repos[0].hooks[0].arguments = [ - "arg_a", - "value_a", - ] - - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("Python") - - assert "arg_a" in result.config_data - assert "value_a" in result.config_data - # Assert the original argument was removed - assert "orig_arg" not in result.config_data - - def test_that_language_config_service_does_nothing_when_pre_commit_settings_is_empty( language_config_service: LanguageConfigService, mock_data_loader: MagicMock, @@ -164,251 +100,7 @@ def mock_loader_side_effect(resource): assert "orig_arg" in result.config_data -def test_that_language_config_service_overrides_arguments_do_not_apply_to_a_different_hook_id( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, - settings_dict: dict, - mock_open: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://sample-repo.com/baddie-finder - hooks: - - id: baddie-finder-hook - args: - - orig_arg - """ - - language_config_service.pre_commit_settings.repos[ - 0 - ].url = "http://sample-repo.com/baddie-finder" - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].id = "goodie-finder-hook" # doesn't match - language_config_service.pre_commit_settings.repos[0].hooks[0].arguments = [ - "arg_a", - "value_a", - ] - - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "arg_a" not in result.config_data - assert "value_a" not in result.config_data - # assert the original arg was left in place - assert "orig_arg" in result.config_data - - -def test_that_language_config_service_adds_additional_arguments_to_a_hook( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, - settings_dict: dict, - mock_open: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://sample-repo.com/baddie-finder - hooks: - - id: baddie-finder-hook - args: - - orig_arg - """ - - language_config_service.pre_commit_settings.repos[ - 0 - ].url = "http://sample-repo.com/baddie-finder" - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].id = "baddie-finder-hook" - language_config_service.pre_commit_settings.repos[0].hooks[0].additional_args = [ - "arg_a", - "value_a", - ] - - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "arg_a" in result.config_data - assert "value_a" in result.config_data - # assert the original arg was left in place - assert "orig_arg" in result.config_data - - -def test_that_language_config_service_adds_additional_arguments_to_a_hook_if_the_hook_did_not_have_any_originally( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, - settings_dict: dict, - mock_open: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://sample-repo.com/baddie-finder - hooks: - - id: baddie-finder-hook - """ - - language_config_service.pre_commit_settings.repos[ - 0 - ].url = "http://sample-repo.com/baddie-finder" - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].id = "baddie-finder-hook" - language_config_service.pre_commit_settings.repos[0].hooks[0].additional_args = [ - "arg_a", - "value_a", - ] - - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "arg_a" in result.config_data - assert "value_a" in result.config_data - - -def test_that_language_config_service_excludes_files_in_specific_hooks( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://example-repo.com/ - rev: 1.0.25 - hooks: - - id: hook-id - - id: hook-id-2 - """ - - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].exclude_file_patterns = ["file_a.py"] - mock_data_loader.side_effect = mock_loader_side_effect - - result_1 = language_config_service.get_language_config("RadLang") - - language_config_service.pre_commit_settings.repos[0].hooks[ - 0 - ].exclude_file_patterns = [] - - result_2 = language_config_service.get_language_config("RadLang") - - assert "file_a" in result_1.config_data - assert "file_a" not in result_2.config_data - - -def test_that_language_config_service_suppresses_hooks_in_repo( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://example-repo.com/ - rev: 1.0.25 - hooks: - - id: hook-id - - id: hook-id-2 - """ - - language_config_service.pre_commit_settings.repos[0].suppressed_hook_ids = [ - "hook-id-2" - ] - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "hook-id-2" not in result.config_data - assert "hook-id" in result.config_data - - -def test_that_language_config_service_removes_repo_when_all_hooks_suppressed( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://example-repo.com/ - rev: 1.0.25 - hooks: - - id: hook-id - - id: hook-id-2 - """ - - language_config_service.pre_commit_settings.repos[0].suppressed_hook_ids = [ - "hook-id", - "hook-id-2", - ] - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "http://example-repo.com/" not in result.config_data - - -def test_that_language_config_service_removes_the_one_hook_multiple_times_without_a_problem( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://example-repo.com/ - rev: 1.0.25 - hooks: - - id: hook-id - """ - - language_config_service.pre_commit_settings.repos[0].suppressed_hook_ids = [ - "hook-id", - "hook-id", - ] - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "hook-id" not in result.config_data - - -def test_that_language_config_service_removes_repo_when_repo_suppressed( - language_config_service: LanguageConfigService, - mock_data_loader: MagicMock, -): - def mock_loader_side_effect(resource): - # Language config file - return """ - repos: - - repo: http://example-repo.com/ - rev: 1.0.25 - hooks: - - id: hook-id - - id: hook-id-2 - """ - - language_config_service.pre_commit_settings.suppressed_repos = [ - "http://example-repo.com/" - ] - mock_data_loader.side_effect = mock_loader_side_effect - - result = language_config_service.get_language_config("RadLang") - - assert "http://example-repo.com/" not in result.config_data - - -#### _load_language_config_files #### +# ### _load_language_config_files #### def test_that_language_config_service_langauge_config_gets_loaded( language_config_service: LanguageConfigService, ): diff --git a/tests/services/test_language_support.py b/tests/services/test_language_support.py index c7ba2b8c..1e86ee82 100644 --- a/tests/services/test_language_support.py +++ b/tests/services/test_language_support.py @@ -1,6 +1,7 @@ from unittest.mock import MagicMock import pytest +from _pytest.python_api import raises from pytest_mock import MockerFixture from secureli.abstractions.pre_commit import ( @@ -98,28 +99,6 @@ def language_support_service( ) -def test_that_language_support_calculates_version_for_language( - language_support_service: LanguageSupportService, - mock_language_config_service: MagicMock, -): - mock_language_config_service.get_language_config.return_value = LanguagePreCommitResult( - language="Python", - version="abc123", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data=""" - repos: - - repo: http://sample-repo.com/baddie-finder - hooks: - - id: baddie-finder - """, - ) - - version = language_support_service.version_for_language(["RadLang"]) - - mock_language_config_service.get_language_config.assert_called_with("base") - assert version is not None - - def test_that_language_support_identifies_a_security_hook_we_can_use_during_init( language_support_service: LanguageSupportService, mock_data_loader: MagicMock, @@ -235,183 +214,6 @@ def mock_loader_side_effect(resource): assert hook_id is None -def test_that_get_current_config_returns_config_data( - language_support_service: LanguageSupportService, mock_open_config: MagicMock -): - config = language_support_service.get_current_configuration() - - assert config["exclude"] == "some-exclude-regex" - - -#### validate_config ##### -def test_that_validate_config_returns_no_output_on_config_match( - language_support_service: LanguageSupportService, - mock_language_config_service: MagicMock, - mock_open_config: MagicMock, - mock_hashlib: MagicMock, -): - config_data = """ - exclude: some-exclude-regex - repos: - - hooks: - - id: some-test-hook - repo: xyz://some-test-repo-url - rev: 1.0.0 - - hooks: - - id: some-other-test-hook - repo: xyz://some-other-test-repo-url - rev: 1.0.0 - """ - - mock_language_config_service.get_language_config.return_value = ( - LanguagePreCommitResult( - language="Python", - version="mock-hash-code", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data=config_data, - ) - ) - - validation_result = language_support_service.validate_config(["Python"]) - - assert validation_result.successful - assert validation_result.output == "" - - -def test_that_validate_config_detects_mismatched_configs( - language_support_service: LanguageSupportService, - mock_language_config_service: MagicMock, - mock_open_config: MagicMock, -): - mock_language_config_service.get_language_config.return_value = LanguagePreCommitResult( - language="Python", - version="abc123", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data='{"exclude": "some-exclude-regex","repos":[{"hooks":[{"id":"some-test-hook"}],"repo":"xyz://some-test-repo-url","rev":"1.0.1"}]}', - ) - validation_result = language_support_service.validate_config(["Python"]) - - assert not validation_result.successful - - -def test_that_validate_config_detects_mismatched_hook_versions( - language_support_service: LanguageSupportService, - mock_hashlib_no_match: MagicMock, - mock_language_config_service: MagicMock, - mock_open_config: MagicMock, -): - config_value = """ - exclude: some-exclude-regex - repos: - - hooks: - - id: some-test-hook - repo: xyz://some-test-repo-url - rev: 1.0.1 - - hooks: - - id: some-other-test-hook - repo: xyz://some-other-test-repo-url - rev: 1.0.0 - """ - - mock_language_config_service.get_language_config.return_value = ( - LanguagePreCommitResult( - language="Python", - version="abc123", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data=config_value, - ) - ) - - validation_result = language_support_service.validate_config(["Python"]) - output_by_line = validation_result.output.splitlines() - - assert ( - output_by_line[-1] - == "Expected xyz://some-test-repo-url to be rev 1.0.1 but it is configured to rev 1.0.0" - ) - - -def test_that_validate_config_detects_extra_repos( - language_support_service: LanguageSupportService, - mock_hashlib_no_match: MagicMock, - mock_language_config_service: MagicMock, - mock_open_config: MagicMock, -): - def mock_side_effect(resource: str): - config_value = """ - exclude: some-exclude-regex - repos: - - hooks: - - id: some-test-hook - repo: xyz://some-test-repo-url - rev: 1.0.0 - """ - if resource == "base": - config_value = """""" - - return LanguagePreCommitResult( - language="Python", - version="abc123", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data=config_value, - ) - - mock_language_config_service.get_language_config.side_effect = mock_side_effect - - validation_result = language_support_service.validate_config(["Python"]) - output_by_line = validation_result.output.splitlines() - - assert output_by_line[-3] == "Found unexpected repos in .pre-commit-config.yaml:" - assert output_by_line[-2] == "- xyz://some-other-test-repo-url" - - -def test_that_validate_config_detects_missing_repos( - language_support_service: LanguageSupportService, - mock_hashlib_no_match: MagicMock, - mock_language_config_service: MagicMock, - mock_open_config: MagicMock, -): - def mock_side_effect(resource: str): - config_value = """ - exclude: some-exclude-regex - repos: - - hooks: - - id: some-test-hook - repo: xyz://some-test-repo-url - rev: 1.0.0 - - hooks: - - id: some-other-test-hook - repo: xyz://some-other-test-repo-url - rev: 1.0.0 - """ - if resource == "base": - config_value = """ - repos: - - hooks: - - id: some-third-test-hook - repo: xyz://some-third-test-repo-url - rev: 1.0.0 - """ - - return LanguagePreCommitResult( - language="Python", - version="abc123", - linter_config=LoadLinterConfigsResult(successful=False, linter_data=list()), - config_data=config_value, - ) - - mock_language_config_service.get_language_config.side_effect = mock_side_effect - - validation_result = language_support_service.validate_config(["Python"]) - output_by_line = validation_result.output.splitlines() - - assert ( - output_by_line[-4] - == "Some expected repos were misssing from .pre-commit-config.yaml:" - ) - assert output_by_line[-3] == "- xyz://some-third-test-repo-url" - - # #### _write_pre_commit_configs #### def test_that_language_support_writes_linter_config_files( language_support_service: LanguageSupportService, @@ -445,5 +247,50 @@ def mock_loader_side_effect(resource): metadata = language_support_service.apply_support(["RadLang"]) - # mock_pre_commit_hook.install.assert_called_once() assert metadata.security_hook_id == "baddie-finder" + + +def test_that_language_support_throws_exception_when_language_config_file_cannot_be_opened( + language_support_service: LanguageSupportService, + mock_language_config_service: MagicMock, + mock_open: MagicMock, +): + mock_language_config_service.get_language_config.return_value = LanguagePreCommitResult( + language="Python", + version="abc123", + linter_config=LoadLinterConfigsResult( + successful=True, + linter_data=[{"key": {"example"}}], + ), + config_data=""" + repos: + - repo: http://sample-repo.com/baddie-finder + hooks: + - id: baddie-finder + """, + ) + + mock_open.side_effect = IOError + + with raises(IOError): + language_support_service.apply_support(["RadLang"]) + + +def test_that_language_support_handles_invalid_language_config( + language_support_service: LanguageSupportService, + mock_language_config_service: MagicMock, +): + mock_language_config_service.get_language_config.return_value = ( + LanguagePreCommitResult( + language="Python", + version="abc123", + linter_config=LoadLinterConfigsResult( + successful=True, + linter_data=[{"key": {"example"}}], + ), + config_data="", + ) + ) + + metadata = language_support_service.apply_support(["RadLang"]) + assert metadata.security_hook_id is None diff --git a/tests/services/test_secureli_ignore.py b/tests/services/test_secureli_ignore.py index c52b54ec..fa9db074 100644 --- a/tests/services/test_secureli_ignore.py +++ b/tests/services/test_secureli_ignore.py @@ -4,7 +4,6 @@ from secureli.settings import ( Settings, RepoFilesSettings, - PreCommitSettings, LanguageSupportSettings, EchoSettings, ) @@ -25,23 +24,16 @@ def language_support() -> LanguageSupportSettings: return LanguageSupportSettings() -@pytest.fixture() -def pre_commit() -> PreCommitSettings: - return PreCommitSettings() - - @pytest.fixture() def settings( repo_files: RepoFilesSettings, echo: EchoSettings, language_support: LanguageSupportSettings, - pre_commit: PreCommitSettings, ) -> Settings: return Settings( repo_files=repo_files, echo=echo, language_support=language_support, - pre_commit=pre_commit, )