Skip to content

Commit

Permalink
feat: Prompted to ReInstall When Running on Branch (#545)
Browse files Browse the repository at this point in the history
secureli-537

<!-- Include general description here -->
Fixed bug that says "seCureLI is installed and up-to-date for the
following language(s)", even though seCureLI is not initialized
completely in that branch. This is an edge case where seCureLI has been
installed on one branch, but not on the other branch in the same
directory due to seCureLI artifacts that are git ignored.

## Changes
<!-- A detailed list of changes -->
* Added messaging and stop of secureli process when this edge case
occurs.
* Updated detect-secret hook version

## 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 tests
* All existing tests are passing

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


<!--
Github-flavored markdown reference:
https://docs.github.com/en/get-started/writing-on-github
-->
  • Loading branch information
isaac-heist-slalom authored May 10, 2024
1 parent 8041462 commit 22bbc35
Show file tree
Hide file tree
Showing 6 changed files with 229 additions and 68 deletions.
2 changes: 1 addition & 1 deletion .secureli/.pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ repos:
hooks:
- id: black
- repo: https://github.com/yelp/detect-secrets
rev: v1.4.0
rev: v1.5.0
hooks:
- id: detect-secrets
83 changes: 49 additions & 34 deletions secureli/actions/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
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 install
from secureli.modules.shared.models import language
from secureli.modules.shared.models.logging import LogAction
from secureli.modules.shared.models.scan import ScanMode
Expand Down Expand Up @@ -68,7 +68,8 @@ def verify_install(
reset: bool,
always_yes: bool,
files: list[Path],
) -> VerifyResult:
action_source: install.ActionSource,
) -> install.VerifyResult:
"""
Installs, upgrades or verifies the current seCureLI installation
:param folder_path: The folder path to initialize the repo for
Expand All @@ -82,9 +83,9 @@ def verify_install(
== ConfigModels.VerifyConfigOutcome.OUT_OF_DATE
):
update_config = self._update_secureli_config_only(always_yes)
if update_config.outcome != VerifyOutcome.UPDATE_SUCCEEDED:
if update_config.outcome != install.VerifyOutcome.UPDATE_SUCCEEDED:
self.action_deps.echo.error("seCureLI could not be verified.")
return VerifyResult(
return install.VerifyResult(
outcome=update_config.outcome,
)
pre_commit_config_location_is_correct = self.action_deps.hooks_scanner.pre_commit.get_pre_commit_config_path_is_correct(
Expand All @@ -100,20 +101,30 @@ def verify_install(
and not pre_commit_config_location_is_correct
)
if pre_commit_to_preserve:
update_result: VerifyResult = (
update_result: install.VerifyResult = (
self._update_secureli_pre_commit_config_location(
folder_path, always_yes
)
)

if update_result.outcome != VerifyOutcome.UPDATE_SUCCEEDED:
if update_result.outcome != install.VerifyOutcome.UPDATE_SUCCEEDED:
self.action_deps.echo.error(
"seCureLI .pre-commit-config.yaml could not be moved."
)
return update_result
else:
preferred_config_path = update_result.file_path

if (
not pre_commit_config_location_is_correct
and not pre_commit_to_preserve
and action_source == install.ActionSource.SCAN
):
self.action_deps.echo.error(
"seCureLI has not been initialized on this branch."
)
return install.VerifyResult(outcome=install.VerifyOutcome.INSTALL_FAILED)

config = self.get_secureli_config(reset=reset)
languages = []

Expand All @@ -124,13 +135,15 @@ def verify_install(
self.action_deps.echo.warning(
f"Newly detected languages are unsupported by seCureLI"
)
return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE, config=config)
return install.VerifyResult(
outcome=install.VerifyOutcome.UP_TO_DATE, config=config
)

self.action_deps.echo.error(
f"seCureLI could not be installed due to an error: {str(e)}"
)
return VerifyResult(
outcome=VerifyOutcome.INSTALL_FAILED,
return install.VerifyResult(
outcome=install.VerifyOutcome.INSTALL_FAILED,
)

newly_detected_languages = [
Expand All @@ -157,8 +170,8 @@ def verify_install(
f"following language(s): {format_sentence_list(languages)}"
)
)
return VerifyResult(
outcome=VerifyOutcome.UP_TO_DATE,
return install.VerifyResult(
outcome=install.VerifyOutcome.UP_TO_DATE,
config=config,
)

Expand All @@ -169,7 +182,7 @@ def _install_secureli(
install_languages: list[str],
always_yes: bool,
pre_commit_config_location: Path = None,
) -> VerifyResult:
) -> install.VerifyResult:
"""
Installs seCureLI into the given folder path and returns the new configuration
:param folder_path: The folder path to initialize the repo for
Expand All @@ -187,12 +200,12 @@ def _install_secureli(
if not should_install:
if new_install:
self.action_deps.echo.error("User canceled install process")
return VerifyResult(
outcome=VerifyOutcome.INSTALL_CANCELED,
return install.VerifyResult(
outcome=install.VerifyOutcome.INSTALL_CANCELED,
)

self.action_deps.echo.warning("Newly detected languages were not installed")
return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE)
return install.VerifyResult(outcome=install.VerifyOutcome.UP_TO_DATE)

settings = self.action_deps.settings.load(folder_path)

Expand Down Expand Up @@ -236,23 +249,23 @@ def _install_secureli(
color=Color.CYAN,
bold=True,
)
return VerifyResult(
outcome=VerifyOutcome.INSTALL_SUCCEEDED,
return install.VerifyResult(
outcome=install.VerifyOutcome.INSTALL_SUCCEEDED,
config=config,
)

def _prompt_to_install(
self, languages: list[str], always_yes: bool, new_install: bool
) -> bool:
"""
Prompts user to determine if secureli should be installed or not
Prompts user to determine if secureli should be initialized or not
:param languages: List of language names to display
:param always_yes: Assume "Yes" to all prompts
:param new_install: Used to determine if the install is new or
if additional languages are being added
"""

new_install_message = "seCureLI has not yet been installed, install now?"
new_install_message = "seCureLI has not yet been initialized, initialize now?"
add_languages_message = (
f"seCureLI has not been installed for the following language(s): "
f"{format_sentence_list(languages)}, install now?"
Expand Down Expand Up @@ -379,7 +392,7 @@ def _prompt_get_lint_config_languages(

return lint_languages

def _update_secureli(self, always_yes: bool) -> VerifyResult:
def _update_secureli(self, always_yes: bool) -> install.VerifyResult:
"""
Prompts the user to update to the latest secureli install.
:param always_yes: Assume "Yes" to all prompts
Expand All @@ -394,41 +407,41 @@ def _update_secureli(self, always_yes: bool) -> VerifyResult:

if not update_confirmed:
self.action_deps.echo.print("\nUpdate declined.\n")
return VerifyResult(outcome=VerifyOutcome.UPDATE_CANCELED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_CANCELED)

update_result = self.action_deps.updater.update()
details = update_result.output
if details:
self.action_deps.echo.print(details)

if update_result.successful:
return VerifyResult(outcome=VerifyOutcome.UPDATE_SUCCEEDED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_SUCCEEDED)
else:
return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_FAILED)

def _update_secureli_config_only(self, always_yes: bool) -> VerifyResult:
def _update_secureli_config_only(self, always_yes: bool) -> install.VerifyResult:
self.action_deps.echo.print("seCureLI is using an out-of-date config.")
response = always_yes or self.action_deps.echo.confirm(
"Update configuration now?",
default_response=True,
)
if not response:
self.action_deps.echo.error("User canceled update process")
return VerifyResult(
outcome=VerifyOutcome.UPDATE_CANCELED,
return install.VerifyResult(
outcome=install.VerifyOutcome.UPDATE_CANCELED,
)

try:
updated_config = self.action_deps.secureli_config.update()
self.action_deps.secureli_config.save(updated_config)

return VerifyResult(outcome=VerifyOutcome.UPDATE_SUCCEEDED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_SUCCEEDED)
except:
return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_FAILED)

def _update_secureli_pre_commit_config_location(
self, folder_path: Path, always_yes: bool
) -> VerifyResult:
) -> install.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.
Expand All @@ -453,20 +466,22 @@ def _update_secureli_pre_commit_config_location(
folder_path
)
)
return VerifyResult(
outcome=VerifyOutcome.UPDATE_SUCCEEDED, file_path=new_file_path
return install.VerifyResult(
outcome=install.VerifyOutcome.UPDATE_SUCCEEDED,
file_path=new_file_path,
)
except:
return VerifyResult(outcome=VerifyOutcome.UPDATE_FAILED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_FAILED)
else:
self.action_deps.echo.warning(".pre-commit-config.yaml migration declined")
deprecated_location = (
self.action_deps.hooks_scanner.pre_commit.get_pre_commit_config_path(
folder_path
)
)
return VerifyResult(
outcome=VerifyOutcome.UPDATE_CANCELED, file_path=deprecated_location
return install.VerifyResult(
outcome=install.VerifyOutcome.UPDATE_CANCELED,
file_path=deprecated_location,
)

def _initial_hooks_update(self, folder_path: Path):
Expand Down
10 changes: 8 additions & 2 deletions secureli/actions/initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from secureli.actions.scan import ScanAction
from secureli.actions.action import Action, ActionDependencies
from secureli.modules.observability.observability_services.logging import LoggingService
from secureli.modules.shared.models.install import VerifyResult
from secureli.modules.shared.models.install import ActionSource, VerifyResult
from secureli.modules.shared.models.logging import LogAction


Expand All @@ -25,7 +25,13 @@ 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, files=None)
verify_result = self.verify_install(
folder_path,
reset,
always_yes,
files=None,
action_source=ActionSource.INITIALIZER,
)
if verify_result.outcome in ScanAction.halting_outcomes:
self.action_deps.logging.failure(LogAction.init, verify_result.outcome)
else:
Expand Down
17 changes: 9 additions & 8 deletions secureli/actions/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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 import install
from secureli.modules.shared.models.logging import LogAction
from secureli.modules.shared.models.publish_results import PublishResultsOption
from secureli.modules.shared.models.result import Result
Expand All @@ -27,10 +27,10 @@ class ScanAction(action.Action):

"""Any verification outcomes that would cause us to not proceed to scan."""
halting_outcomes = [
VerifyOutcome.INSTALL_FAILED,
VerifyOutcome.INSTALL_CANCELED,
VerifyOutcome.UPDATE_FAILED,
VerifyOutcome.UPDATE_CANCELED,
install.VerifyOutcome.INSTALL_FAILED,
install.VerifyOutcome.INSTALL_CANCELED,
install.VerifyOutcome.UPDATE_FAILED,
install.VerifyOutcome.UPDATE_CANCELED,
]

def __init__(
Expand Down Expand Up @@ -99,6 +99,7 @@ def scan_repo(
False,
always_yes,
scan_files,
action_source=install.ActionSource.SCAN,
)

# Check if pre-commit hooks are up-to-date
Expand Down Expand Up @@ -160,7 +161,7 @@ def scan_repo(
else:
sys.exit(ExitCode.SCAN_ISSUES_DETECTED.value)

def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult:
def _check_secureli_hook_updates(self, folder_path: Path) -> install.VerifyResult:
"""
Queries repositories referenced by pre-commit hooks to check
if we have the latest revisions listed in the .pre-commit-config.yaml file
Expand All @@ -178,7 +179,7 @@ def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult:

if not repos_to_update:
self.action_deps.echo.print("No hooks to update")
return VerifyResult(outcome=VerifyOutcome.UP_TO_DATE)
return install.VerifyResult(outcome=install.VerifyOutcome.UP_TO_DATE)

for repo, revs in repos_to_update.items():
self.action_deps.echo.debug(
Expand All @@ -188,7 +189,7 @@ def _check_secureli_hook_updates(self, folder_path: Path) -> VerifyResult:
"You have out-of-date pre-commit hooks. Run `secureli update` to update them."
)
# Since we don't actually perform the updates here, return an outcome of UPDATE_CANCELLED
return VerifyResult(outcome=VerifyOutcome.UPDATE_CANCELED)
return install.VerifyResult(outcome=install.VerifyOutcome.UPDATE_CANCELED)

def _get_commited_files(self, scan_mode: ScanMode) -> list[Path]:
"""
Expand Down
5 changes: 5 additions & 0 deletions secureli/modules/shared/models/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ class VerifyOutcome(str, Enum):
UP_TO_DATE = "up-to-date"


class ActionSource(str, Enum):
INITIALIZER = "initializer"
SCAN = "scan"


class VerifyResult(pydantic.BaseModel):
"""
The outcomes of performing verification. Actions can use these results
Expand Down
Loading

0 comments on commit 22bbc35

Please sign in to comment.