Skip to content

Commit

Permalink
Merge branch 'refactor/secureli-000-modular-refactor' into feature/se…
Browse files Browse the repository at this point in the history
…cureli-435-pii-from-scratch
  • Loading branch information
Kathleen Hogan committed Mar 22, 2024
2 parents 52b1eb3 + fbd983d commit 192b2ce
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 38 deletions.
24 changes: 14 additions & 10 deletions secureli/actions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,24 @@ class Action(ABC):
def __init__(self, action_deps: ActionDependencies):
self.action_deps = action_deps

def get_secureli_config(self, reset: bool) -> secureli_config.SecureliConfig:
return (
secureli_config.SecureliConfig()
if reset
else self.action_deps.secureli_config.load()
)

def verify_install(
self, folder_path: Path, reset: bool, always_yes: bool
self, folder_path: Path, reset: bool, always_yes: bool, files: list[Path]
) -> VerifyResult:
"""
Installs, upgrades or verifies the current seCureLI installation
:param folder_path: The folder path to initialize the repo for
:param reset: If true, disregard existing configuration and start fresh
:param always_yes: Assume "Yes" to all prompts
:param files: A List of files to scope the install to. This allows language
detection to run on only a selected list of files when scanning the repo.
"""

if (
self.action_deps.secureli_config.verify()
== secureli_config.VerifyConfigOutcome.OUT_OF_DATE
Expand Down Expand Up @@ -84,15 +92,11 @@ def verify_install(
)
return update_result

config = (
secureli_config.SecureliConfig()
if reset
else self.action_deps.secureli_config.load()
)
config = self.get_secureli_config(reset=reset)
languages = []

try:
languages = self._detect_languages(folder_path)
languages = self._detect_languages(folder_path, files)
except (ValueError, language.LanguageNotSupportedError) as e:
if config.languages and config.version_installed:
self.action_deps.echo.warning(
Expand Down Expand Up @@ -296,14 +300,14 @@ def _run_post_install_scan(
f"{format_sentence_list(config.languages)} does not support secrets detection, skipping"
)

def _detect_languages(self, folder_path: Path) -> list[str]:
def _detect_languages(self, folder_path: Path, files: list[Path]) -> list[str]:
"""
Detects programming languages present in the repository
:param folder_path: The folder path to initialize the repo for
:return: A list of all languages found in the repository
"""

analyze_result = self.action_deps.language_analyzer.analyze(folder_path)
analyze_result = self.action_deps.language_analyzer.analyze(folder_path, files)

if analyze_result.skipped_files:
self.action_deps.echo.warning(
Expand Down
2 changes: 1 addition & 1 deletion secureli/actions/initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def initialize_repo(
:param reset: If true, disregard existing configuration and start fresh
:param always_yes: Assume "Yes" to all prompts
"""
verify_result = self.verify_install(folder_path, reset, always_yes)
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)
else:
Expand Down
33 changes: 32 additions & 1 deletion secureli/actions/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
from pathlib import Path
from time import time
from typing import Optional
from git import Repo

from secureli.modules.shared.abstractions.echo import EchoAbstraction
from secureli.actions import action
from secureli.modules.shared.abstractions.repo import GitRepo
from secureli.modules.shared.models.exit_codes import ExitCode
from secureli.modules.shared.models.install import VerifyOutcome, VerifyResult
from secureli.modules.shared.models.logging import LogAction
Expand Down Expand Up @@ -37,12 +39,14 @@ def __init__(
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 _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult:
"""
Expand Down Expand Up @@ -98,6 +102,24 @@ def publish_results(
else:
self.logging.failure(LogAction.publish, result.result_message)

def get_commited_files(self, scan_mode: ScanMode) -> list[Path]:
"""
Attempts to build a list of commited files for use in language detection if
the user is scanning staged files for an existing installation
:param scan_mode: Determines which files are scanned in the repo (i.e. staged only or all)
:returns: a list of Path objects for the commited files
"""
config = self.get_secureli_config(reset=False)
installed = bool(config.languages and config.version_installed)

if not installed or scan_mode != ScanMode.STAGED_ONLY:
return None
try:
committed_files = self.git_repo.get_commit_diff()
return [Path(file) for file in committed_files]
except:
return None

def scan_repo(
self,
folder_path: Path,
Expand All @@ -117,7 +139,16 @@ def scan_repo(
:param specific_test: If set, limits scanning to the single pre-commit hook.
Otherwise, scans with all hooks.
"""
verify_result = self.verify_install(folder_path, False, always_yes)

scan_files = [Path(file) for file in files or []] or self.get_commited_files(
scan_mode
)
verify_result = self.verify_install(
folder_path,
False,
always_yes,
scan_files,
)

# Check if pre-commit hooks are up-to-date
secureli_config = self.action_deps.secureli_config.load()
Expand Down
5 changes: 5 additions & 0 deletions secureli/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from secureli.actions.scan import ScanAction
from secureli.actions.build import BuildAction
from secureli.actions.update import UpdateAction
from secureli.modules.shared.abstractions.repo import GitRepo
from secureli.repositories.repo_files import RepoFilesRepository
from secureli.repositories.secureli_config import SecureliConfigRepository
from secureli.repositories.repo_settings import SecureliRepository
Expand Down Expand Up @@ -83,6 +84,9 @@ class Container(containers.DeclarativeContainer):
echo=echo,
)

"""Wraps the execution and management of git commands"""
git_repo = providers.Factory(GitRepo)

# Services

"""Analyzes a set of files to try to determine the most common languages"""
Expand Down Expand Up @@ -180,6 +184,7 @@ class Container(containers.DeclarativeContainer):
logging=logging_service,
hooks_scanner=hooks_scanner_service,
pii_scanner=pii_scanner_service,
git_repo=git_repo,
)

"""Update Action, representing what happens when the update command is invoked"""
Expand Down
4 changes: 2 additions & 2 deletions secureli/modules/language_analyzer/language_analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def __init__(
self.repo_files = repo_files
self.lexer_guesser = lexer_guesser

def analyze(self, folder_path: Path) -> AnalyzeResult:
def analyze(self, folder_path: Path, files: list[Path]) -> AnalyzeResult:
"""
Analyzes the folder structure and lists languages found
:param folder_path: The path to the repository to analyze
Expand All @@ -29,7 +29,7 @@ def analyze(self, folder_path: Path) -> AnalyzeResult:
40% of the repo is JavaScript, the result will be a dictionary containing keys
"Python" and "JavaScript" with values 0.6 and 0.4 respectively
"""
file_paths = self.repo_files.list_repo_files(folder_path)
file_paths = files if files else self.repo_files.list_repo_files(folder_path)
results = defaultdict(int)

skipped_files = []
Expand Down
21 changes: 21 additions & 0 deletions secureli/modules/shared/abstractions/repo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from abc import ABC, abstractmethod
import git


class RepoAbstraction(ABC):
"""
Abstracts the configuring and execution of git repo features.
"""

@abstractmethod
def get_commit_diff(self) -> list[str]:
pass


class GitRepo(RepoAbstraction):
"""
Implementation and wrapper around git repo features
"""

def get_commit_diff(self) -> list[str]:
return git.Repo().head.commit.diff()
46 changes: 25 additions & 21 deletions tests/actions/test_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_that_initialize_repo_raises_value_error_without_any_supported_languages
language_proportions={}, skipped_files=[]
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_echo.error.assert_called_with(
"seCureLI could not be installed due to an error: No supported languages found in current repository"
Expand All @@ -83,7 +83,7 @@ def test_that_initialize_repo_install_flow_selects_both_languages(
skipped_files=[],
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_echo.print.assert_called_with(
"seCureLI has been installed successfully for the following language(s): RadLang and CoolLang.\n",
Expand All @@ -105,7 +105,7 @@ def test_that_initialize_repo_install_flow_performs_security_analysis(
skipped_files=[],
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_hooks_scanner.scan_repo.assert_called_once()

Expand All @@ -118,7 +118,7 @@ def test_that_initialize_repo_install_flow_displays_security_analysis_results(
output="Detect secrets...Failed",
failures=[ScanFailure(repo="repo", id="id", file="file")],
)
action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

action_deps.echo.print.assert_any_call("Detect secrets...Failed")

Expand All @@ -140,7 +140,7 @@ def test_that_initialize_repo_install_flow_skips_security_analysis_if_unavailabl
version="abc123", security_hook_id=None
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_hooks_scanner.scan_repo.assert_not_called()

Expand Down Expand Up @@ -171,7 +171,7 @@ def test_that_initialize_repo_install_flow_warns_about_skipped_files(
successful=True, backup_hook_path=None
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

assert (
mock_echo.warning.call_count == 3
Expand All @@ -184,7 +184,7 @@ def test_that_initialize_repo_can_be_canceled(
):
mock_echo.confirm.return_value = False

action.verify_install(test_folder_path, reset=True, always_yes=False)
action.verify_install(test_folder_path, reset=True, always_yes=False, files=None)

mock_echo.error.assert_called_with("User canceled install process")

Expand All @@ -205,7 +205,7 @@ def test_that_initialize_repo_selects_previously_selected_language(
)
mock_language_support.version_for_language.return_value = "abc123"

action.verify_install(test_folder_path, reset=False, always_yes=True)
action.verify_install(test_folder_path, reset=False, always_yes=True, files=None)

mock_echo.print.assert_called_with(
"seCureLI is installed and up-to-date for the following language(s): PreviousLang"
Expand All @@ -223,7 +223,7 @@ def test_that_initialize_repo_prompts_to_upgrade_config_if_old_schema(
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)
action.verify_install(test_folder_path, reset=False, always_yes=False, files=None)

mock_echo.error.assert_called_with("seCureLI could not be verified.")

Expand All @@ -250,7 +250,9 @@ def test_that_initialize_repo_updates_repo_config_if_old_schema(

mock_language_support.version_for_language.return_value = "abc123"

result = action.verify_install(test_folder_path, reset=False, always_yes=True)
result = action.verify_install(
test_folder_path, reset=False, always_yes=True, files=None
)

assert result.outcome == VerifyOutcome.UP_TO_DATE

Expand All @@ -265,7 +267,7 @@ def test_that_initialize_repo_reports_errors_when_schema_update_fails(

mock_secureli_config.update.side_effect = Exception

action.verify_install(test_folder_path, reset=False, always_yes=True)
action.verify_install(test_folder_path, reset=False, always_yes=True, files=None)

mock_echo.error.assert_called_with("seCureLI could not be verified.")

Expand All @@ -280,7 +282,7 @@ def test_that_initialize_repo_is_aborted_by_the_user_if_the_process_is_canceled(
mock_echo.confirm.return_value = False
mock_secureli_config.load.return_value = SecureliConfig() # fresh config

action.verify_install(test_folder_path, reset=False, always_yes=False)
action.verify_install(test_folder_path, reset=False, always_yes=False, files=None)

mock_echo.error.assert_called_with("User canceled install process")

Expand All @@ -300,7 +302,9 @@ def test_that_initialize_repo_returns_up_to_date_if_the_process_is_canceled_on_e
language_proportions={"RadLang": 0.5, "CoolLang": 0.5}, skipped_files=[]
)

result = action.verify_install(test_folder_path, reset=False, always_yes=False)
result = action.verify_install(
test_folder_path, reset=False, always_yes=False, files=None
)
assert result.outcome == VerifyOutcome.UP_TO_DATE


Expand All @@ -322,7 +326,7 @@ def test_that_initialize_repo_prints_warnings_for_failed_linter_config_writes(
successful=True, backup_hook_path=None
)

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_echo.warning.assert_called_once_with(config_write_error)

Expand All @@ -341,7 +345,7 @@ def test_that_verify_install_returns_failed_result_on_new_install_language_not_s
)

verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=False
test_folder_path, reset=False, always_yes=False, files=None
)

assert verify_result.outcome == VerifyOutcome.INSTALL_FAILED
Expand All @@ -361,7 +365,7 @@ def test_that_verify_install_returns_up_to_date_result_on_existing_install_langu
)

verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=False
test_folder_path, reset=False, always_yes=False, files=None
)

assert verify_result.outcome == VerifyOutcome.UP_TO_DATE
Expand All @@ -381,7 +385,7 @@ def test_that_verify_install_returns_up_to_date_result_on_existing_install_no_ne
)

verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=False
test_folder_path, reset=False, always_yes=False, files=None
)

assert verify_result.outcome == VerifyOutcome.UP_TO_DATE
Expand All @@ -401,7 +405,7 @@ def test_that_verify_install_returns_success_result_newly_detected_language_inst
)

verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=True
test_folder_path, reset=False, always_yes=True, files=None
)

assert verify_result.outcome == VerifyOutcome.INSTALL_SUCCEEDED
Expand All @@ -420,7 +424,7 @@ def test_that_verify_install_returns_failure_result_without_re_commit_config_fil
"ERROR"
)
verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=True
test_folder_path, reset=False, always_yes=True, files=None
)
mock_echo.error.assert_called_once_with(
"seCureLI pre-commit-config.yaml could not be updated."
Expand All @@ -438,7 +442,7 @@ def test_that_verify_install_continues_after_pre_commit_config_file_moved(
test_folder_path / ".secureli" / ".pre-commit-config.yaml"
)
verify_result = action.verify_install(
test_folder_path, reset=False, always_yes=True
test_folder_path, reset=False, always_yes=True, files=None
)
assert verify_result.outcome == VerifyOutcome.INSTALL_SUCCEEDED

Expand Down Expand Up @@ -512,7 +516,7 @@ def test_that_initialize_repo_install_flow_warns_about_overwriting_pre_commit_fi

mock_updater.pre_commit.install.return_value = install_result

action.verify_install(test_folder_path, reset=True, always_yes=True)
action.verify_install(test_folder_path, reset=True, always_yes=True, files=None)

mock_echo.warning.assert_called_once_with(
(
Expand Down
Loading

0 comments on commit 192b2ce

Please sign in to comment.