Feat: use custom pre commit config in refactor (#479)

secureli-XXX

including recent feature from main branch to maintain settings if repo
has existing pre-commit-config.yaml on secureli init.


## Changes
<!-- A detailed list of changes -->
*Updates to pre-commit, action, scan, and language support to check for
existing congif yaml, and copy relevant contents into newly generated
.secureli/pre-commit-config.yaml as well as lots of new tests.

## Testing
<!--
- Existing tests pass
- coverage is maintained at 100% to account for new functionality
- end-to-end tests passing (including new e2e test)
- Optionally, manually test new feature by making a simple test repo
that has a pre-commit-config.yaml in it before running secureli against
it, double check that existing repos and other settings are maintained
in new config file
 -->
*

## Clean Code Checklist
<!-- This is here to support you. Some/most checkboxes may not apply to
your change -->
- [x] Meets acceptance criteria for issue
- [x] New logic is covered with automated tests
- [ ] Appropriate exception handling added
- [x] Thoughtful logging included
- [ ] Documentation is updated
- [ ] Follow-up work is documented in TODOs
- [ ] TODOs have a ticket associated with them
- [x] No commented-out code included


<!--
Github-flavored markdown reference:
https://docs.github.com/en/get-started/writing-on-github
-->

---------

Co-authored-by: Jordan Heffernan <[email protected]>

.gitignore

@@ -244,5 +244,6 @@ fabric.properties
 *.drawio.dtmp
 
 # Secureli-generated files (do not modify):
-.secureli
+.secureli/logs
+.secureli/repo-config.yaml
 # End Secureli-generated files

.secureli.yaml

@@ -9,3 +9,5 @@ repo_files:
   - .png
   - .jpg
   max_file_size: 1000000
+telemetry:
+  api_url: https://log-api.newrelic.com/log/v1

.vscode/extensions.json

@@ -0,0 +1,7 @@
+{
+    "recommendations": [
+        "ms-python.python",
+        "ms-python.vscode-pylance",
+        "ms-python.black-formatter"
+    ]
+}

.vscode/launch.json

@@ -10,7 +10,7 @@
             "stopOnEntry": false,
             "console": "integratedTerminal",
             "justMyCode": true,
-            "args": ["--version"]
+            "args": ["--version", "update"]
         },
         {
             "name": "Python: secureli --help",
@@ -55,6 +55,15 @@
             "console": "integratedTerminal",
             "justMyCode": true,
             "args": ["update"]
+        },
+        {
+            "name": "Debug Tests",
+            "type": "debugpy",
+            "request": "launch",
+            "program": "${file}",
+            "purpose": ["debug-test"],
+            "console": "integratedTerminal",
+            "justMyCode": false
         }
     ]
 }

pyproject.toml

@@ -25,9 +25,9 @@ init = ["install", "secureli_init"]
 secureli_init = "secureli init -y"
 install = "poetry install"
 lint = "black --check ."
-precommit = "pre-commit run -a"
+precommit = "pre-commit run --config .secureli/.pre-commit-config.yaml --all-files"
 test = ["init", "lint", "coverage_run", "coverage_report"]
-e2e = "bats --verbose-run tests/end-to-end/test.bats"
+e2e = "bats --verbose-run tests/end-to-end"
 
 [tool.poetry.dependencies]
 # Until `python-dependency-injector` supports python 3.12, restrict to python 3.11 and lower

secureli/actions/action.py

@@ -63,16 +63,35 @@ def verify_install(
         ):
             update_config = self._update_secureli_config_only(always_yes)
             if update_config.outcome != VerifyOutcome.UPDATE_SUCCEEDED:
-                self.action_deps.echo.error(f"seCureLI could not be verified.")
+                self.action_deps.echo.error("seCureLI could not be verified.")
                 return VerifyResult(
                     outcome=update_config.outcome,
                 )
 
+        pre_commit_config_location = (
+            self.action_deps.scanner.pre_commit.get_preferred_pre_commit_config_path(
+                folder_path
+            )
+        )
+        if not pre_commit_config_location.exists():
+            update_result: VerifyResult = (
+                self._update_secureli_pre_commit_config_location(
+                    folder_path, always_yes
+                )
+            )
+            pre_commit_config_location = update_result.file_path
+            if update_result.outcome != VerifyOutcome.UPDATE_SUCCEEDED:
+                self.action_deps.echo.error(
+                    "seCureLI pre-commit-config.yaml could not be updated."
+                )
+                return update_result
+
         config = (
             secureli_config.SecureliConfig()
             if reset
             else self.action_deps.secureli_config.load()
         )
+        languages = []
 
         try:
             languages = self._detect_languages(folder_path)
@@ -101,7 +120,11 @@ def verify_install(
             or newly_detected_languages
         ):
             return self._install_secureli(
-                folder_path, languages, newly_detected_languages, always_yes
+                folder_path,
+                languages,
+                newly_detected_languages,
+                always_yes,
+                pre_commit_config_location,
             )
         else:
             self.action_deps.echo.print(
@@ -121,6 +144,7 @@ def _install_secureli(
         detected_languages: list[str],
         install_languages: list[str],
         always_yes: bool,
+        pre_commit_config_location: Path = None,
     ) -> VerifyResult:
         """
         Installs seCureLI into the given folder path and returns the new configuration
@@ -155,7 +179,7 @@ def _install_secureli(
         )
         language_config_result = (
             self.action_deps.language_support.build_pre_commit_config(
-                install_languages, lint_languages
+                install_languages, lint_languages, pre_commit_config_location
             )
         )
         metadata = self.action_deps.language_support.apply_support(
@@ -375,3 +399,42 @@ def _update_secureli_config_only(self, always_yes: bool) -> VerifyResult:
             return VerifyResult(outcome=VerifyOutcome.UPDATE_SUCCEEDED)
         except:
             return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED)
+
+    def _update_secureli_pre_commit_config_location(
+        self, folder_path: Path, always_yes: bool
+    ) -> VerifyResult:
+        """
+        In order to provide an upgrade path for existing users of secureli,
+        we will prompt users to move their .pre-commit-config.yaml into the .secureli/ directory.
+        I would consider this particular implementation to be technical debt but it is consistent with
+        an existing pattern for upgrading the secureli config file schema.
+        Once this has existed for awhile, we could remove this function altogether since
+        we make no promises of about backward compatibility with our pre-release versions.
+        Long term, I think there are better ways to implement one-time upgrade migrations
+        to avoid breaking backward compatibility.
+        """
+        self.action_deps.echo.print(
+            "seCureLI's .pre-commit-config.yaml is in a deprecated location."
+        )
+        response = always_yes or self.action_deps.echo.confirm(
+            "Would you like it automatically moved to the .secureli/ directory?",
+            default_response=True,
+        )
+        if response:
+            try:
+                new_file_path = self.action_deps.scanner.pre_commit.migrate_config_file(
+                    folder_path
+                )
+                return VerifyResult(
+                    outcome=VerifyOutcome.UPDATE_SUCCEEDED, file_path=new_file_path
+                )
+            except:
+                return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED)
+        else:
+            self.action_deps.echo.warning(".pre-commit-config.yaml migration declined")
+            deprecated_location = self.action_deps.scanner.get_pre_commit_config_path(
+                folder_path
+            )
+            return VerifyResult(
+                outcome=VerifyOutcome.UPDATE_CANCELED, file_path=deprecated_location
+            )

secureli/actions/scan.py

@@ -45,18 +45,18 @@ def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult:
         """
         Queries repositories referenced by pre-commit hooks to check
         if we have the latest revisions listed in the .pre-commit-config.yaml file
-        :param folder_path: The folder path containing the .pre-commit-config.yaml file
+        :param folder_path: The folder path containing the .secureli/ folder
         """
 
-        self.action_deps.echo.info("Checking for pre-commit hook updates...")
+        self.action_deps.echo.print("Checking for pre-commit hook updates...")
         pre_commit_config = self.scanner.pre_commit.get_pre_commit_config(folder_path)
 
         repos_to_update = self.scanner.pre_commit.check_for_hook_updates(
             pre_commit_config
         )
 
         if not repos_to_update:
-            self.action_deps.echo.info("No hooks to update")
+            self.action_deps.echo.print("No hooks to update")
             return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE)
 
         for repo, revs in repos_to_update.items():

secureli/container.py

@@ -79,6 +79,7 @@ class Container(containers.DeclarativeContainer):
     pre_commit_abstraction = providers.Factory(
         PreCommitAbstraction,
         command_timeout_seconds=config.language_support.command_timeout_seconds,
+        echo=echo,
     )
 
     # Services
@@ -107,6 +108,7 @@ class Container(containers.DeclarativeContainer):
         git_ignore=git_ignore_service,
         language_config=language_config_service,
         data_loader=read_resource,
+        echo=echo,
     )
 
     """Analyzes a given repo to try to identify the most common language"""

secureli/modules/core/core_services/scanner.py

@@ -67,7 +67,7 @@ def _parse_scan_ouput(self, folder_path: Path, output: str = "") -> ScanOuput:
         """
         Parses the output from a scan and returns a list of Failure objects representing any
         hook rule failures during a scan.
-        :param folder_path: folder containing .pre-commit-config.yaml, usually repository root
+        :param folder_path: folder containing .secureli folder, usually repository root
         :param output: Raw output from a scan.
         :return: ScanOuput object representing a list of hook rule Failure objects.
         """

secureli/modules/core/core_services/updater.py

@@ -65,7 +65,7 @@ def update(self, folder_path: Path = Path(".")):
         :param folder_path: Indicates the git folder against which you run secureli
         :return: ExecuteResult, indicating success or failure.
         """
-        update_message = "Updating .pre-commit-config.yaml...\n"
+        update_message = "Updating pre-commit hooks...\n"
         output = update_message
 
         hook_install_result = self.pre_commit.update(folder_path)

secureli/modules/language_analyzer/git_ignore.py

@@ -16,7 +16,7 @@ class GitIgnoreService:
     """
 
     header = "# Secureli-generated files (do not modify):"
-    ignore_entries = [".secureli"]
+    ignore_entries = [".secureli/logs", ".secureli/repo-config.yaml"]
     footer = "# End Secureli-generated files"
 
     def ignore_secureli_files(self):

secureli/modules/language_analyzer/language_support.py

@@ -7,6 +7,7 @@
 
 import secureli.repositories.secureli_config as SecureliConfig
 from secureli.modules.shared.abstractions.pre_commit import PreCommitAbstraction
+from secureli.modules.shared.abstractions.echo import EchoAbstraction
 from secureli.modules.language_analyzer import git_ignore, language_config
 from secureli.modules.shared.utilities import hash_config
 
@@ -23,11 +24,13 @@ def __init__(
         language_config: language_config.LanguageConfigService,
         git_ignore: git_ignore.GitIgnoreService,
         data_loader: Callable[[str], str],
+        echo: EchoAbstraction,
     ):
         self.git_ignore = git_ignore
         self.pre_commit_hook = pre_commit_hook
         self.language_config = language_config
         self.data_loader = data_loader
+        self.echo = echo
 
     def apply_support(
         self,
@@ -45,8 +48,8 @@ def apply_support(
         as well as a secret-detection hook ID, if present.
         """
 
-        path_to_pre_commit_file = Path(
-            SecureliConfig.FOLDER_PATH / ".pre-commit-config.yaml"
+        path_to_pre_commit_file: Path = self.pre_commit_hook.get_pre_commit_config_path(
+            SecureliConfig.FOLDER_PATH
         )
 
         linter_config_write_result = self._write_pre_commit_configs(
@@ -129,21 +132,43 @@ def get_configuration(self, languages: list[str]) -> HookConfiguration:
         return HookConfiguration(repos=repos)
 
     def build_pre_commit_config(
-        self, languages: list[str], lint_languages: Iterable[str]
+        self,
+        languages: list[str],
+        lint_languages: Iterable[str],
+        pre_commit_config_location: Optional[Path] = None,
     ) -> language.BuildConfigResult:
         """
         Builds the final .pre-commit-config.yaml from all supported repo languages. Also returns any and all
         linter configuration data.
         :param languages: list of languages to get calculated configuration for.
         :param lint_languages: list of languages to add lint pre-commit hooks for.
-        :return: BuildConfigResult
+        :return: language.BuildConfigResult
         """
-        config_data = []
+        config_repos = []
+        existing_data = {}
         successful_languages: list[str] = []
         linter_configs: list[LinterConfig] = []
         config_languages = [*languages, "base"]
         config_lint_languages = [*lint_languages, "base"]
 
+        if pre_commit_config_location:
+            with open(pre_commit_config_location) as stream:
+                try:
+                    data = yaml.safe_load(stream)
+                    existing_data = data or {}
+                    config_repos += data["repos"]
+                except yaml.YAMLError:
+                    self.echo.error(
+                        f"There was an issue parsing existing pre-commit-config.yaml."
+                    )
+                    return language.BuildConfigResult(
+                        successful=False,
+                        languages_added=[],
+                        config_data={},
+                        version="",
+                        linter_configs=linter_configs,
+                    )
+
         for config_language in config_languages:
             include_linter = config_language in config_lint_languages
             result = self.language_config.get_language_config(
@@ -162,13 +187,13 @@ def build_pre_commit_config(
                     else None
                 )
                 data = yaml.safe_load(result.config_data)
-                config_data += data["repos"] or []
+                config_repos += data["repos"] or []
 
-        config = {"repos": config_data}
+        config = {**existing_data, "repos": config_repos}
         version = hash_config(yaml.dump(config))
 
         return language.BuildConfigResult(
-            successful=True if len(config_data) > 0 else False,
+            successful=True if len(config_repos) > 0 else False,
             languages_added=successful_languages,
             config_data=config,
             version=version,