feat: Update template file hook before running initial scan (#521)

secureli-433

<!-- Include general description here -->
Update template file hook before running initial scan, not after

## Changes
<!-- A detailed list of changes -->
* Update template file hook before running initial scan
* Added logging to action dependencies 
* Cleaned up update progress output (`pre-commit gc` not accepting
`--config` flag)
* Cleaned up container dependency injection
* Updated test to get rid of warnings in terminal
* Updated hooks versions


## Testing
<!--
Mention updated tests and any manual testing performed.
Are aspects not yet tested or not easily testable?
Feel free to include screenshots if appropriate.
 -->
* Added required unit test
* Updated existing unit tests and all passing

## 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
-->

.secureli/.pre-commit-config.yaml

@@ -1,18 +1,18 @@
-# See https://pre-commit.com for more information
-# See https://pre-commit.com/hooks.html for more hooks
-repos:
--   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
-    hooks:
-    -   id: trailing-whitespace
-    -   id: end-of-file-fixer
-    -   id: check-yaml
-    -   id: check-added-large-files
--   repo: https://github.com/psf/black-pre-commit-mirror
-    rev: 24.4.1
-    hooks:
-    -   id: black
--   repo: https://github.com/yelp/detect-secrets
-    rev: v1.4.0
-    hooks:
-    -   id: detect-secrets
+# See https://pre-commit.com for more information
+# See https://pre-commit.com/hooks.html for more hooks
+repos:
+    - repo: https://github.com/pre-commit/pre-commit-hooks
+      rev: v4.6.0
+      hooks:
+          - id: trailing-whitespace
+          - id: end-of-file-fixer
+          - id: check-yaml
+          - id: check-added-large-files
+    - repo: https://github.com/psf/black-pre-commit-mirror
+      rev: 24.4.2
+      hooks:
+          - id: black
+    - repo: https://github.com/yelp/detect-secrets
+      rev: v1.4.0
+      hooks:
+          - id: detect-secrets

secureli/actions/action.py

@@ -1,10 +1,14 @@
 from abc import ABC
 from pathlib import Path
+from rich.progress import Progress
+
+from secureli.modules.observability.observability_services.logging import LoggingService
 from secureli.modules.shared.abstractions.echo import EchoAbstraction
 from secureli.modules.observability.consts.logging import TELEMETRY_DEFAULT_ENDPOINT
 from secureli.modules.shared.models.echo import Color
 from secureli.modules.shared.models.install import VerifyOutcome, VerifyResult
 from secureli.modules.shared.models import language
+from secureli.modules.shared.models.logging import LogAction
 from secureli.modules.shared.models.scan import ScanMode
 from secureli.modules.language_analyzer import language_analyzer, language_support
 from secureli.modules.core.core_services.scanner import HooksScannerService
@@ -33,6 +37,7 @@ def __init__(
         secureli_config: secureli_config.SecureliConfigRepository,
         settings: SecureliRepository,
         updater: UpdaterService,
+        logging: LoggingService,
     ):
         self.echo = echo
         self.language_analyzer = language_analyzer
@@ -41,6 +46,7 @@ def __init__(
         self.secureli_config = secureli_config
         self.settings = settings
         self.updater = updater
+        self.logging = logging
 
 
 class Action(ABC):
@@ -296,6 +302,9 @@ def _run_post_install_scan(
                     )
                 )
 
+        # Updated hooks prior to initial scan
+        self._initial_hooks_update(folder_path)
+
         if secret_test_id := metadata.security_hook_id:
             self.action_deps.echo.print(
                 f"The following language(s) support secrets detection: {format_sentence_list(config.languages)}"
@@ -459,3 +468,24 @@ def _update_secureli_pre_commit_config_location(
             return VerifyResult(
                 outcome=VerifyOutcome.UPDATE_CANCELED, file_path=deprecated_location
             )
+
+    def _initial_hooks_update(self, folder_path: Path):
+        with Progress(transient=True) as progress:
+            self.action_deps.echo.print(
+                "Updating hooks to the latest version prior to initial scan..."
+            )
+            progress.add_task("Updating...", start=False, total=None)
+            update_result = self.action_deps.updater.update_hooks(folder_path)
+            details = (
+                update_result.output
+                or "Unknown output while updating hooks to latest version"
+            )
+            progress.stop()
+            self.action_deps.echo.print(details)
+            if not update_result.successful:
+                self.action_deps.logging.failure(LogAction.update, details)
+            else:
+                self.action_deps.echo.print(
+                    "Hooks successfully updated to latest version"
+                )
+                self.action_deps.logging.success(LogAction.update)

secureli/actions/initializer.py

@@ -13,10 +13,8 @@ class InitializerAction(Action):
     def __init__(
         self,
         action_deps: ActionDependencies,
-        logging: LoggingService,
     ):
         super().__init__(action_deps)
-        self.logging = logging
 
     def initialize_repo(
         self, folder_path: Path, reset: bool, always_yes: bool
@@ -29,8 +27,8 @@ def initialize_repo(
         """
         verify_result = self.verify_install(folder_path, reset, always_yes, files=None)
         if verify_result.outcome in ScanAction.halting_outcomes:
-            self.logging.failure(LogAction.init, verify_result.outcome)
+            self.action_deps.logging.failure(LogAction.init, verify_result.outcome)
         else:
-            self.logging.success(LogAction.init)
+            self.action_deps.logging.success(LogAction.init)
 
         return verify_result

secureli/actions/scan.py

@@ -36,17 +36,13 @@ class ScanAction(action.Action):
     def __init__(
         self,
         action_deps: action.ActionDependencies,
-        echo: EchoAbstraction,
-        logging: LoggingService,
         hooks_scanner: HooksScannerService,
         pii_scanner: PiiScannerService,
         git_repo: GitRepo,
     ):
         super().__init__(action_deps)
         self.hooks_scanner = hooks_scanner
         self.pii_scanner = pii_scanner
-        self.echo = echo
-        self.logging = logging
         self.git_repo = git_repo
 
     def publish_results(
@@ -66,12 +62,14 @@ def publish_results(
             and not action_successful
         ):
             result = utilities.post_log(log_str, Settings())
-            self.echo.debug(result.result_message)
+            self.action_deps.echo.debug(result.result_message)
 
             if result.result == Result.SUCCESS:
-                self.logging.success(LogAction.publish)
+                self.action_deps.logging.success(LogAction.publish)
             else:
-                self.logging.failure(LogAction.publish, result.result_message)
+                self.action_deps.logging.failure(
+                    LogAction.publish, result.result_message
+                )
 
     def scan_repo(
         self,
@@ -129,7 +127,7 @@ def scan_repo(
         scan_result = utilities.merge_scan_results([pii_scan_result, hooks_scan_result])
 
         details = scan_result.output or "Unknown output during scan"
-        self.echo.print(details)
+        self.action_deps.echo.print(details)
 
         failure_count = len(scan_result.failures)
         scan_result_failures_json_string = json.dumps(
@@ -141,9 +139,9 @@ def scan_repo(
         )
 
         log_data = (
-            self.logging.success(LogAction.scan)
+            self.action_deps.logging.success(LogAction.scan)
             if scan_result.successful
-            else self.logging.failure(
+            else self.action_deps.logging.failure(
                 LogAction.scan,
                 scan_result_failures_json_string,
                 failure_count,
@@ -156,7 +154,9 @@ def scan_repo(
             log_str=log_data.json(exclude_none=True),
         )
         if scan_result.successful:
-            self.echo.print("Scan executed successfully and detected no issues!")
+            self.action_deps.echo.print(
+                "Scan executed successfully and detected no issues!"
+            )
         else:
             sys.exit(ExitCode.SCAN_ISSUES_DETECTED.value)
 

secureli/actions/update.py

@@ -13,13 +13,9 @@ class UpdateAction(Action):
     def __init__(
         self,
         action_deps: ActionDependencies,
-        echo: EchoAbstraction,
-        logging: LoggingService,
         updater: UpdaterService,
     ):
         super().__init__(action_deps)
-        self.echo = echo
-        self.logging = logging
         self.updater = updater
 
     def update_hooks(self, folder_path: Path, latest: Optional[bool] = False):
@@ -32,35 +28,35 @@ def update_hooks(self, folder_path: Path, latest: Optional[bool] = False):
         """
         with Progress(transient=True) as progress:
             if latest:
-                self.echo.print("Updating hooks to the latest version...")
+                self.action_deps.echo.print("Updating hooks to the latest version...")
                 progress.add_task("Updating...", start=False, total=None)
                 update_result = self.updater.update_hooks(folder_path)
                 details = (
                     update_result.output
                     or "Unknown output while updating hooks to latest version"
                 )
-                self.echo.print(details)
+                progress.stop()
+                self.action_deps.echo.print(details)
                 if not update_result.successful:
-                    self.echo.print(details)
-                    progress.stop()
-                    self.logging.failure(LogAction.update, details)
+                    self.action_deps.echo.print(details)
+                    self.action_deps.logging.failure(LogAction.update, details)
                 else:
-                    self.echo.print("Hooks successfully updated to latest version")
-                    progress.stop()
-                    self.logging.success(LogAction.update)
+                    self.action_deps.echo.print(
+                        "Hooks successfully updated to latest version"
+                    )
+                    self.action_deps.logging.success(LogAction.update)
             else:
-                self.echo.print("Beginning update...")
+                self.action_deps.echo.print("Beginning update...")
                 progress.add_task("Updating...", start=False, total=None)
                 install_result = self.updater.update(folder_path)
                 details = (
                     install_result.output or "Unknown output during hook installation"
                 )
-                self.echo.print(details)
+                progress.stop()
+                self.action_deps.echo.print(details)
                 if not install_result.successful:
-                    self.echo.print(details)
-                    progress.stop()
-                    self.logging.failure(LogAction.update, details)
+                    self.action_deps.echo.print(details)
+                    self.action_deps.logging.failure(LogAction.update, details)
                 else:
-                    self.echo.print("Update executed successfully.")
-                    progress.stop()
-                    self.logging.success(LogAction.update)
+                    self.action_deps.echo.print("Update executed successfully.")
+                    self.action_deps.logging.success(LogAction.update)

secureli/container.py

@@ -160,6 +160,7 @@ class Container(containers.DeclarativeContainer):
         secureli_config=secureli_config_repository,
         settings=settings_repository,
         updater=updater_service,
+        logging=logging_service,
     )
 
     """The Build Action, used to render the build_data using the echo"""
@@ -174,15 +175,12 @@ class Container(containers.DeclarativeContainer):
     initializer_action = providers.Factory(
         InitializerAction,
         action_deps=action_deps,
-        logging=logging_service,
     )
 
     """Scan Action, representing what happens when the scan command is invoked"""
     scan_action = providers.Factory(
         ScanAction,
         action_deps=action_deps,
-        echo=echo,
-        logging=logging_service,
         hooks_scanner=hooks_scanner_service,
         pii_scanner=pii_scanner_service,
         git_repo=git_repo,
@@ -192,7 +190,5 @@ class Container(containers.DeclarativeContainer):
     update_action = providers.Factory(
         UpdateAction,
         action_deps=action_deps,
-        echo=echo,
-        logging=logging_service,
         updater=updater_service,
     )

secureli/main.py

@@ -101,7 +101,6 @@ def init(
         Path(directory), reset, yes
     )
     if init_result.outcome in [
-        VerifyOutcome.INSTALL_SUCCEEDED,
         VerifyOutcome.UP_TO_DATE,
         VerifyOutcome.UPDATE_SUCCEEDED,
     ]:

secureli/modules/shared/abstractions/pre_commit.py

@@ -296,8 +296,6 @@ def remove_unused_hooks(self, folder_path: Path) -> ExecuteResult:
         subprocess_args = [
             "pre-commit",
             "gc",
-            "--config",
-            self.get_pre_commit_config_path(folder_path),
             "--color",
             "always",
         ]

tests/actions/test_action.py

@@ -9,6 +9,7 @@
 from secureli.modules.shared.models.echo import Color
 from secureli.modules.shared.models.install import VerifyOutcome
 from secureli.modules.shared.models import language
+from secureli.modules.shared.models.logging import LogAction
 import secureli.modules.shared.models.repository as RepositoryModels
 import secureli.modules.shared.models.config as ConfigModels
 from secureli.modules.shared.models.scan import ScanFailure, ScanResult
@@ -38,6 +39,7 @@ def action_deps(
     mock_secureli_config: MagicMock,
     mock_updater: MagicMock,
     mock_settings: MagicMock,
+    mock_logging_service: MagicMock,
 ) -> ActionDependencies:
     return ActionDependencies(
         mock_echo,
@@ -47,6 +49,7 @@ def action_deps(
         mock_secureli_config,
         mock_settings,
         mock_updater,
+        mock_logging_service,
     )
 
 
@@ -768,3 +771,18 @@ def test_that_prompt_get_telemetry_api_url_returns_prompt_response(
     result = action._prompt_get_telemetry_api_url(False)
 
     assert result is mock_api_endpoint
+
+
+def test_that_unsuccessful_initial_hooks_update_logs_details(
+    action: Action, action_deps: ActionDependencies
+):
+    action_deps.updater.update_hooks.return_value = UpdateResult(
+        successful=False, output="Update failed"
+    )
+
+    action._initial_hooks_update(Path("test/path"))
+
+    logging_result = action_deps.logging
+
+    assert logging_result.failure
+    logging_result.failure.assert_called_once_with(LogAction.update, "Update failed")