From d525896604689324c7c3e231c33a8a085b682dda Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 17 Aug 2026 13:30:12 +0530 Subject: [PATCH 1/5] Add terraform-app-setup with API v2 defaults and Min-Commander-Version gate --- keepercommander/command_categories.py | 3 +- keepercommander/commands/start_service.py | 3 + .../service/commands/service_docker_setup.py | 155 +----------------- .../service/commands/terraform_app_setup.py | 112 +++++++++++++ .../decorators/min_commander_version.py | 96 +++++++++++ keepercommander/service/decorators/unified.py | 4 +- keepercommander/service/docker/__init__.py | 5 +- keepercommander/service/docker/setup_base.py | 135 +++++++++++++++ .../service/test_min_commander_version.py | 138 ++++++++++++++++ .../service/test_terraform_app_setup.py | 121 ++++++++++++++ 10 files changed, 620 insertions(+), 152 deletions(-) create mode 100644 keepercommander/service/commands/terraform_app_setup.py create mode 100644 keepercommander/service/decorators/min_commander_version.py create mode 100644 unit-tests/service/test_min_commander_version.py create mode 100644 unit-tests/service/test_terraform_app_setup.py diff --git a/keepercommander/command_categories.py b/keepercommander/command_categories.py index c2a6f6b54..b0ae463de 100644 --- a/keepercommander/command_categories.py +++ b/keepercommander/command_categories.py @@ -83,7 +83,8 @@ # Service Mode REST API 'Service Mode REST API': { 'service-create', 'service-add-config', 'service-start', 'service-stop', 'service-status', - 'service-config-add', 'service-docker-setup', 'slack-app-setup', 'teams-app-setup', + 'service-config-add', 'service-docker-setup', 'terraform-app-setup', + 'slack-app-setup', 'teams-app-setup', 'sailpoint-app-setup', 'gchat-app-setup' }, diff --git a/keepercommander/commands/start_service.py b/keepercommander/commands/start_service.py index 0d7eaded9..4037acc38 100644 --- a/keepercommander/commands/start_service.py +++ b/keepercommander/commands/start_service.py @@ -13,6 +13,7 @@ from ..service.commands.config_operation import AddConfigService from ..service.commands.handle_service import StartService, StopService, ServiceStatus from ..service.commands.service_docker_setup import ServiceDockerSetupCommand +from ..service.commands.terraform_app_setup import TerraformAppSetupCommand from ..service.commands.integrations import ( GChatAppSetupCommand, SlackAppSetupCommand, @@ -27,6 +28,7 @@ def register_commands(commands): commands['service-stop'] = StopService() commands['service-status'] = ServiceStatus() commands['service-docker-setup'] = ServiceDockerSetupCommand() + commands['terraform-app-setup'] = TerraformAppSetupCommand() commands['slack-app-setup'] = SlackAppSetupCommand() commands['teams-app-setup'] = TeamsAppSetupCommand() commands['sailpoint-app-setup'] = SailPointAppSetupCommand() @@ -40,6 +42,7 @@ def register_command_info(aliases, command_info): StopService, ServiceStatus, ServiceDockerSetupCommand, + TerraformAppSetupCommand, SlackAppSetupCommand, TeamsAppSetupCommand, SailPointAppSetupCommand, diff --git a/keepercommander/service/commands/service_docker_setup.py b/keepercommander/service/commands/service_docker_setup.py index f3795b1ef..e94d6a835 100644 --- a/keepercommander/service/commands/service_docker_setup.py +++ b/keepercommander/service/commands/service_docker_setup.py @@ -16,11 +16,9 @@ import argparse import os from dataclasses import asdict -from typing import Dict, Any from ...commands.base import Command, raise_parse_exception, suppress_exit from ...display import bcolors -from ...error import CommandError from ..config.config_validation import ConfigValidator, ValidationError from ..docker import ( DockerSetupBase, DockerSetupConstants, DockerSetupPrinter, @@ -69,19 +67,17 @@ def get_parser(self): def execute(self, params, **kwargs): """Main execution flow for standalone command""" - self._require_file_based_config(params, 'service-docker-setup') + command_name = self.get_parser().prog + self._require_file_based_config(params, command_name) - # Parse arguments config_path = self._require_commander_config_file( - 'service-docker-setup', + command_name, kwargs.get('config_path'), params, ) - - # Print header + DockerSetupPrinter.print_header("Docker Setup") - - # Run core setup steps (inherited from DockerSetupBase) + setup_result = self.run_setup_steps( params=params, folder_name=kwargs.get('folder_name', DockerSetupConstants.DEFAULT_FOLDER_NAME), @@ -91,19 +87,13 @@ def execute(self, params, **kwargs): timeout=kwargs.get('timeout', DockerSetupConstants.DEFAULT_TIMEOUT), skip_device_setup=kwargs.get('skip_device_setup', False) ) - - # Get service configuration + DockerSetupPrinter.print_completion("Docker Setup Complete!") service_config = self.get_service_configuration(params) - - # Generate docker-compose.yml + self.generate_and_save_docker_compose(setup_result, service_config) DockerSetupPrinter.print_completion("Service Mode Configuration Complete!") - - # Print success message self.print_standalone_success_message(setup_result, service_config, config_path) - - return def get_service_configuration(self, params) -> ServiceConfig: """Interactively get service configuration from user""" @@ -221,134 +211,3 @@ def _get_queue_config(self) -> bool: print(f" Queue mode enables async API (v2) for better performance") queue_input = input(f"{bcolors.OKBLUE}Enable queue mode? [Press Enter for Yes] (y/n):{bcolors.ENDC} ").strip().lower() return queue_input != 'n' - - - def _get_advanced_security_config(self) -> Dict[str, Any]: - """Get advanced security configuration""" - print(f"\n{bcolors.BOLD}Advanced Security (optional):{bcolors.ENDC}") - print(f" Configure IP filtering, rate limiting, and response encryption") - enable_advanced = input(f"{bcolors.OKBLUE}Enable advanced security? [Press Enter for No] (y/n):{bcolors.ENDC} ").strip().lower() == 'y' - - config = { - 'allowed_ip': '0.0.0.0/0,::/0', - 'denied_ip': '', - 'rate_limit': '', - 'encryption_enabled': False, - 'encryption_key': '', - 'token_expiration': '' - } - - if enable_advanced: - # IP Allowed List - config.update(self._get_ip_allowed_config()) - - # IP Denied List - config.update(self._get_ip_denied_config()) - - # Rate Limiting - config.update(self._get_rate_limit_config()) - - # Encryption - config.update(self._get_encryption_config()) - - # Token Expiration - config.update(self._get_token_expiration_config()) - - return config - - def _get_ip_allowed_config(self) -> Dict[str, str]: - """Get allowed IP configuration""" - print(f"\n{bcolors.BOLD}IP Allowed List:{bcolors.ENDC}") - print(f" Comma-separated IPs or CIDR ranges (e.g., 192.168.1.0/24,10.0.0.1)") - - ip_list = input(f"{bcolors.OKBLUE}Allowed IPs [Press Enter for all]:{bcolors.ENDC} ").strip() - - if ip_list: - while True: - try: - return {'allowed_ip': ConfigValidator.validate_ip_list(ip_list)} - except ValidationError as e: - print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") - ip_list = input(f"{bcolors.OKBLUE}Allowed IPs [Press Enter for all]:{bcolors.ENDC} ").strip() - if not ip_list: - break - - return {'allowed_ip': '0.0.0.0/0,::/0'} - - def _get_ip_denied_config(self) -> Dict[str, str]: - """Get denied IP configuration""" - print(f"\n{bcolors.BOLD}IP Denied List:{bcolors.ENDC}") - print(f" Comma-separated IPs or CIDR ranges to block") - - ip_list = input(f"{bcolors.OKBLUE}Denied IPs [Press Enter to skip]:{bcolors.ENDC} ").strip() - - if ip_list: - while True: - try: - return {'denied_ip': ConfigValidator.validate_ip_list(ip_list)} - except ValidationError as e: - print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") - ip_list = input(f"{bcolors.OKBLUE}Denied IPs [Press Enter to skip]:{bcolors.ENDC} ").strip() - if not ip_list: - break - - return {'denied_ip': ''} - - def _get_rate_limit_config(self) -> Dict[str, str]: - """Get rate limiting configuration""" - print(f"\n{bcolors.BOLD}Rate Limiting:{bcolors.ENDC}") - print(f" Format: / (e.g., 10/minute, 100/hour, 1000/day)") - - rate_limit = input(f"{bcolors.OKBLUE}Rate limit [Press Enter to skip]:{bcolors.ENDC} ").strip() - - if rate_limit: - while True: - try: - return {'rate_limit': ConfigValidator.validate_rate_limit(rate_limit)} - except ValidationError as e: - print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") - rate_limit = input(f"{bcolors.OKBLUE}Rate limit [Press Enter to skip]:{bcolors.ENDC} ").strip() - if not rate_limit: - break - - return {'rate_limit': ''} - - def _get_encryption_config(self) -> Dict[str, Any]: - """Get encryption configuration""" - print(f"\n{bcolors.BOLD}Response Encryption:{bcolors.ENDC}") - print(f" Enable AES-256 encryption for API responses") - enable_encryption = input(f"{bcolors.OKBLUE}Enable encryption? [Press Enter for No] (y/n):{bcolors.ENDC} ").strip().lower() == 'y' - - config = {'encryption_enabled': enable_encryption, 'encryption_key': ''} - - if enable_encryption: - print(f" Encryption key must be exactly 32 alphanumeric characters") - while True: - key = input(f"{bcolors.OKBLUE}Encryption key (32 chars):{bcolors.ENDC} ").strip() - try: - config['encryption_key'] = ConfigValidator.validate_encryption_key(key) - break - except ValidationError as e: - print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") - - return config - - def _get_token_expiration_config(self) -> Dict[str, str]: - """Get token expiration configuration""" - print(f"\n{bcolors.BOLD}API Token Expiration:{bcolors.ENDC}") - print(f" Format: Xm (minutes), Xh (hours), Xd (days) - e.g., 30m, 24h, 7d") - - expiration = input(f"{bcolors.OKBLUE}Token expiration [Press Enter for never]:{bcolors.ENDC} ").strip() - - if expiration: - while True: - try: - ConfigValidator.parse_expiration_time(expiration) - return {'token_expiration': expiration} - except ValidationError as e: - print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") - expiration = input(f"{bcolors.OKBLUE}Token expiration [Press Enter for never]:{bcolors.ENDC} ").strip() - if not expiration: - break - - return {'token_expiration': ''} diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py new file mode 100644 index 000000000..a9046b89b --- /dev/null +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -0,0 +1,112 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' str: + try: + return RuntimeServiceConfig().validate_command_list( + TerraformSetupConstants.SERVICE_COMMANDS, params + ) + except ValidationError as e: + raise CommandError( + self.get_parser().prog, + f'Terraform command allowlist validation failed: {e}', + ) + + def _get_queue_config(self) -> bool: + return True diff --git a/keepercommander/service/decorators/min_commander_version.py b/keepercommander/service/decorators/min_commander_version.py new file mode 100644 index 000000000..88bdaae43 --- /dev/null +++ b/keepercommander/service/decorators/min_commander_version.py @@ -0,0 +1,96 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' Optional[str]: + value = request.headers.get(MIN_COMMANDER_VERSION_HEADER) + if value is None: + return None + value = str(value).strip() + return value or None + + +def _parse_version(version_str: str) -> Optional[Version]: + if not version_str or len(version_str) > 64: + return None + try: + return Version(version_str.lstrip('vV')) + except InvalidVersion: + return None + + +def check_min_commander_version() -> Optional[Tuple[dict, int]]: + """ + If Min-Commander-Version is present, require running Commander >= that version. + + Missing header: no-op. + Invalid header: 400. + Too old: 426 with upgrade guidance. + """ + required_raw = _read_min_commander_version_header() + if required_raw is None: + return None + + required = _parse_version(required_raw) + if required is None: + return { + 'status': 'error', + 'error': ( + f'Invalid {MIN_COMMANDER_VERSION_HEADER} header. ' + 'Expected a dotted version such as 18.1.0.' + ), + }, 400 + + current_raw = str(commander_version).strip() + current = _parse_version(current_raw) + if current is None: + logger.error('Unable to parse running Commander version') + return { + 'status': 'error', + 'error': 'Unable to determine running Commander version.', + }, 500 + + if current >= required: + return None + + message = ( + f'Commander version {current_raw} is below the required minimum {required_raw}. ' + f'Please update Keeper Commander and re-run terraform-app-setup.' + ) + logger.info(message) + return {'status': 'error', 'error': message}, 426 + + +def min_commander_version_check(fn): + """Run after auth: enforce Min-Commander-Version when the header is present.""" + + @wraps(fn) + def wrapper(*args, **kwargs): + version_error = check_min_commander_version() + if version_error: + return version_error + return fn(*args, **kwargs) + + return wrapper diff --git a/keepercommander/service/decorators/unified.py b/keepercommander/service/decorators/unified.py index dc87e14d3..873d3cc3c 100644 --- a/keepercommander/service/decorators/unified.py +++ b/keepercommander/service/decorators/unified.py @@ -15,6 +15,7 @@ from .api_logging import api_log_handler from .security import security_check from .auth import auth_check, policy_check +from .min_commander_version import min_commander_version_check def unified_api_decorator() -> Callable: def decorator(f: Callable) -> Callable: @@ -22,10 +23,11 @@ def decorator(f: Callable) -> Callable: @api_log_handler @security_check @auth_check + @min_commander_version_check @policy_check @catch_all @debug_decorator def wrapped_function(*args, **kwargs): return f(*args, **kwargs) return wrapped_function - return decorator \ No newline at end of file + return decorator diff --git a/keepercommander/service/docker/__init__.py b/keepercommander/service/docker/__init__.py index c30acd0e6..cc03a716c 100644 --- a/keepercommander/service/docker/__init__.py +++ b/keepercommander/service/docker/__init__.py @@ -20,8 +20,9 @@ """ from .models import ( - DockerSetupConstants, SetupResult, ServiceConfig, SlackConfig, TeamsConfig, - SailPointConfig, GChatConfig, GChatConstants, SetupStep, ApproverTeam, ApprovalsConfig, + DockerSetupConstants, SetupResult, ServiceConfig, + SlackConfig, TeamsConfig, SailPointConfig, GChatConfig, GChatConstants, + SetupStep, ApproverTeam, ApprovalsConfig, ) from .printer import DockerSetupPrinter from .setup_base import DockerSetupBase diff --git a/keepercommander/service/docker/setup_base.py b/keepercommander/service/docker/setup_base.py index 48fbb07bb..d43eea9a6 100644 --- a/keepercommander/service/docker/setup_base.py +++ b/keepercommander/service/docker/setup_base.py @@ -584,3 +584,138 @@ def _get_cloudflare_config(self) -> Dict[str, Any]: print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") return config + + def _get_advanced_security_config(self) -> Dict[str, Any]: + """Get advanced security configuration (IP filter, rate limit, encryption, token expiry).""" + print(f"\n{bcolors.BOLD}Advanced Security (optional):{bcolors.ENDC}") + print(f" Configure IP filtering, rate limiting, and response encryption") + enable_advanced = input( + f"{bcolors.OKBLUE}Enable advanced security? [Press Enter for No] (y/n):{bcolors.ENDC} " + ).strip().lower() == 'y' + + config = { + 'allowed_ip': '0.0.0.0/0,::/0', + 'denied_ip': '', + 'rate_limit': '', + 'encryption_enabled': False, + 'encryption_key': '', + 'token_expiration': '' + } + + if enable_advanced: + config.update(self._get_ip_allowed_config()) + config.update(self._get_ip_denied_config()) + config.update(self._get_rate_limit_config()) + config.update(self._get_encryption_config()) + config.update(self._get_token_expiration_config()) + + return config + + def _get_ip_allowed_config(self) -> Dict[str, str]: + """Get allowed IP configuration""" + print(f"\n{bcolors.BOLD}IP Allowed List:{bcolors.ENDC}") + print(f" Comma-separated IPs or CIDR ranges (e.g., 192.168.1.0/24,10.0.0.1)") + + ip_list = input(f"{bcolors.OKBLUE}Allowed IPs [Press Enter for all]:{bcolors.ENDC} ").strip() + + if ip_list: + while True: + try: + return {'allowed_ip': ConfigValidator.validate_ip_list(ip_list)} + except ValidationError as e: + print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") + ip_list = input( + f"{bcolors.OKBLUE}Allowed IPs [Press Enter for all]:{bcolors.ENDC} " + ).strip() + if not ip_list: + break + + return {'allowed_ip': '0.0.0.0/0,::/0'} + + def _get_ip_denied_config(self) -> Dict[str, str]: + """Get denied IP configuration""" + print(f"\n{bcolors.BOLD}IP Denied List:{bcolors.ENDC}") + print(f" Comma-separated IPs or CIDR ranges to block") + + ip_list = input(f"{bcolors.OKBLUE}Denied IPs [Press Enter to skip]:{bcolors.ENDC} ").strip() + + if ip_list: + while True: + try: + return {'denied_ip': ConfigValidator.validate_ip_list(ip_list)} + except ValidationError as e: + print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") + ip_list = input( + f"{bcolors.OKBLUE}Denied IPs [Press Enter to skip]:{bcolors.ENDC} " + ).strip() + if not ip_list: + break + + return {'denied_ip': ''} + + def _get_rate_limit_config(self) -> Dict[str, str]: + """Get rate limiting configuration""" + print(f"\n{bcolors.BOLD}Rate Limiting:{bcolors.ENDC}") + print(f" Format: / (e.g., 10/minute, 100/hour, 1000/day)") + + rate_limit = input(f"{bcolors.OKBLUE}Rate limit [Press Enter to skip]:{bcolors.ENDC} ").strip() + + if rate_limit: + while True: + try: + return {'rate_limit': ConfigValidator.validate_rate_limit(rate_limit)} + except ValidationError as e: + print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") + rate_limit = input( + f"{bcolors.OKBLUE}Rate limit [Press Enter to skip]:{bcolors.ENDC} " + ).strip() + if not rate_limit: + break + + return {'rate_limit': ''} + + def _get_encryption_config(self) -> Dict[str, Any]: + """Get encryption configuration""" + print(f"\n{bcolors.BOLD}Response Encryption:{bcolors.ENDC}") + print(f" Enable AES-256 encryption for API responses") + enable_encryption = input( + f"{bcolors.OKBLUE}Enable encryption? [Press Enter for No] (y/n):{bcolors.ENDC} " + ).strip().lower() == 'y' + + config = {'encryption_enabled': enable_encryption, 'encryption_key': ''} + + if enable_encryption: + print(f" Encryption key must be exactly 32 alphanumeric characters") + while True: + key = input(f"{bcolors.OKBLUE}Encryption key (32 chars):{bcolors.ENDC} ").strip() + try: + config['encryption_key'] = ConfigValidator.validate_encryption_key(key) + break + except ValidationError as e: + print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") + + return config + + def _get_token_expiration_config(self) -> Dict[str, str]: + """Get token expiration configuration""" + print(f"\n{bcolors.BOLD}API Token Expiration:{bcolors.ENDC}") + print(f" Format: Xm (minutes), Xh (hours), Xd (days) - e.g., 30m, 24h, 7d") + + expiration = input( + f"{bcolors.OKBLUE}Token expiration [Press Enter for never]:{bcolors.ENDC} " + ).strip() + + if expiration: + while True: + try: + ConfigValidator.parse_expiration_time(expiration) + return {'token_expiration': expiration} + except ValidationError as e: + print(f"{bcolors.FAIL}Error: {str(e)}{bcolors.ENDC}") + expiration = input( + f"{bcolors.OKBLUE}Token expiration [Press Enter for never]:{bcolors.ENDC} " + ).strip() + if not expiration: + break + + return {'token_expiration': ''} diff --git a/unit-tests/service/test_min_commander_version.py b/unit-tests/service/test_min_commander_version.py new file mode 100644 index 000000000..4824c0b6f --- /dev/null +++ b/unit-tests/service/test_min_commander_version.py @@ -0,0 +1,138 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' Date: Mon, 17 Aug 2026 13:56:14 +0530 Subject: [PATCH 2/5] Change error message --- .../decorators/min_commander_version.py | 2 +- .../service/test_min_commander_version.py | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/keepercommander/service/decorators/min_commander_version.py b/keepercommander/service/decorators/min_commander_version.py index 88bdaae43..c945b9fd3 100644 --- a/keepercommander/service/decorators/min_commander_version.py +++ b/keepercommander/service/decorators/min_commander_version.py @@ -77,7 +77,7 @@ def check_min_commander_version() -> Optional[Tuple[dict, int]]: message = ( f'Commander version {current_raw} is below the required minimum {required_raw}. ' - f'Please update Keeper Commander and re-run terraform-app-setup.' + f'Please update Keeper Commander to >= {required_raw} and retry.' ) logger.info(message) return {'status': 'error', 'error': message}, 426 diff --git a/unit-tests/service/test_min_commander_version.py b/unit-tests/service/test_min_commander_version.py index 4824c0b6f..8b60dd91f 100644 --- a/unit-tests/service/test_min_commander_version.py +++ b/unit-tests/service/test_min_commander_version.py @@ -98,7 +98,8 @@ def test_rejects_when_current_too_old(self): self.assertEqual(body['status'], 'error') self.assertIn('17.0.0', body['error']) self.assertIn('18.1.0', body['error']) - self.assertIn('terraform-app-setup', body['error']) + self.assertIn('Please update Keeper Commander to >= 18.1.0 and retry.', body['error']) + self.assertNotIn('terraform-app-setup', body['error']) @mock.patch( 'keepercommander.service.decorators.min_commander_version.commander_version', @@ -115,6 +116,21 @@ def test_rejects_invalid_header(self): self.assertEqual(body['status'], 'error') self.assertIn('Invalid', body['error']) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version.commander_version', + 'not-a-version', + ) + def test_rejects_when_running_version_unparseable(self): + with self.app.test_request_context( + '/api/v2/executecommand-async', + method='POST', + headers={MIN_COMMANDER_VERSION_HEADER: '18.1.0'}, + ): + body, status = check_min_commander_version() + self.assertEqual(status, 500) + self.assertEqual(body['status'], 'error') + self.assertIn('Unable to determine running Commander version', body['error']) + @mock.patch( 'keepercommander.service.decorators.min_commander_version.commander_version', '17.0.0', From 57da611d823ece9867b3f161ab8e9997ba379b16 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 17 Aug 2026 14:41:52 +0530 Subject: [PATCH 3/5] Separate commands as tuple and improve test cases --- .../service/commands/terraform_app_setup.py | 36 +++++++---- .../decorators/min_commander_version.py | 28 +++++---- setup.cfg | 1 + .../service/test_min_commander_version.py | 39 ++++++++---- .../service/test_terraform_app_setup.py | 59 ++++++++++++------- 5 files changed, 107 insertions(+), 56 deletions(-) diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py index a9046b89b..74790e9ea 100644 --- a/keepercommander/service/commands/terraform_app_setup.py +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -28,17 +28,18 @@ class TerraformSetupConstants: DEFAULT_RECORD_NAME = 'Commander Service Mode Terraform Config' DEFAULT_TIMEOUT = DockerSetupConstants.DEFAULT_TIMEOUT - SERVICE_COMMANDS = ( - 'this-device,sync-down,switch-to-mc,switch-to-msp,' - 'msp-add,msp-down,msp-info,msp-remove,msp-update,' - 'enterprise-info,enterprise-node,enterprise-user,enterprise-role,' - 'enterprise-team,enterprise-down,enterprise-push,team-approve,' - 'record-add,record-update,rm,get,list,record-type-info,' - 'share-folder,rmdir,rndir,mkdir,epm,scim,mv,pam,secrets-manager,' - 'ln,share-record,' - 'nsf-mkdir,nsf-get,nsf-rmdir,nsf-record-add,nsf-record-update,' - 'nsf-rm,nsf-rndir,nsf-share-folder,nsf-share-record,nsf-ln' + SERVICE_COMMANDS_LIST = ( + 'this-device', 'sync-down', 'switch-to-mc', 'switch-to-msp', + 'msp-add', 'msp-down', 'msp-info', 'msp-remove', 'msp-update', + 'enterprise-info', 'enterprise-node', 'enterprise-user', 'enterprise-role', + 'enterprise-team', 'enterprise-down', 'enterprise-push', 'team-approve', + 'record-add', 'record-update', 'rm', 'get', 'list', 'record-type-info', + 'share-folder', 'rmdir', 'rndir', 'mkdir', 'epm', 'scim', 'mv', 'pam', + 'secrets-manager', 'ln', 'share-record', + 'nsf-mkdir', 'nsf-get', 'nsf-rmdir', 'nsf-record-add', 'nsf-record-update', + 'nsf-rm', 'nsf-rndir', 'nsf-share-folder', 'nsf-share-record', 'nsf-ln', ) + SERVICE_COMMANDS = ','.join(SERVICE_COMMANDS_LIST) terraform_app_setup_parser = argparse.ArgumentParser( @@ -91,13 +92,18 @@ def get_parser(self): return terraform_app_setup_parser def execute(self, params, **kwargs): + # setdefault covers programmatic execute() without argparse defaults. kwargs.setdefault('folder_name', TerraformSetupConstants.DEFAULT_FOLDER_NAME) kwargs.setdefault('app_name', TerraformSetupConstants.DEFAULT_APP_NAME) kwargs.setdefault('record_name', TerraformSetupConstants.DEFAULT_RECORD_NAME) kwargs.setdefault('timeout', TerraformSetupConstants.DEFAULT_TIMEOUT) - return super().execute(params, **kwargs) + self._validated_commands = self._validate_terraform_commands(params) + try: + return super().execute(params, **kwargs) + finally: + self._validated_commands = None - def _get_commands_config(self, params) -> str: + def _validate_terraform_commands(self, params) -> str: try: return RuntimeServiceConfig().validate_command_list( TerraformSetupConstants.SERVICE_COMMANDS, params @@ -108,5 +114,11 @@ def _get_commands_config(self, params) -> str: f'Terraform command allowlist validation failed: {e}', ) + def _get_commands_config(self, params) -> str: + cached = getattr(self, '_validated_commands', None) + if cached is not None: + return cached + return self._validate_terraform_commands(params) + def _get_queue_config(self) -> bool: return True diff --git a/keepercommander/service/decorators/min_commander_version.py b/keepercommander/service/decorators/min_commander_version.py index c945b9fd3..3b3068f17 100644 --- a/keepercommander/service/decorators/min_commander_version.py +++ b/keepercommander/service/decorators/min_commander_version.py @@ -24,14 +24,6 @@ MIN_COMMANDER_VERSION_HEADER = 'Min-Commander-Version' -def _read_min_commander_version_header() -> Optional[str]: - value = request.headers.get(MIN_COMMANDER_VERSION_HEADER) - if value is None: - return None - value = str(value).strip() - return value or None - - def _parse_version(version_str: str) -> Optional[Version]: if not version_str or len(version_str) > 64: return None @@ -41,6 +33,18 @@ def _parse_version(version_str: str) -> Optional[Version]: return None +_RUNNING_VERSION_RAW = str(commander_version).strip() +_RUNNING_VERSION = _parse_version(_RUNNING_VERSION_RAW) + + +def _read_min_commander_version_header() -> Optional[str]: + value = request.headers.get(MIN_COMMANDER_VERSION_HEADER) + if value is None: + return None + value = value.strip() + return value or None + + def check_min_commander_version() -> Optional[Tuple[dict, int]]: """ If Min-Commander-Version is present, require running Commander >= that version. @@ -63,20 +67,18 @@ def check_min_commander_version() -> Optional[Tuple[dict, int]]: ), }, 400 - current_raw = str(commander_version).strip() - current = _parse_version(current_raw) - if current is None: + if _RUNNING_VERSION is None: logger.error('Unable to parse running Commander version') return { 'status': 'error', 'error': 'Unable to determine running Commander version.', }, 500 - if current >= required: + if _RUNNING_VERSION >= required: return None message = ( - f'Commander version {current_raw} is below the required minimum {required_raw}. ' + f'Commander version {_RUNNING_VERSION_RAW} is below the required minimum {required_raw}. ' f'Please update Keeper Commander to >= {required_raw} and retry.' ) logger.info(message) diff --git a/setup.cfg b/setup.cfg index 51de347d0..6b0f81bbc 100644 --- a/setup.cfg +++ b/setup.cfg @@ -43,6 +43,7 @@ install_requires = prompt_toolkit protobuf>=5.29.6,<6 googleapis-common-protos + packaging psutil pycryptodomex>=3.20.0 pyngrok diff --git a/unit-tests/service/test_min_commander_version.py b/unit-tests/service/test_min_commander_version.py index 8b60dd91f..2a0b415e1 100644 --- a/unit-tests/service/test_min_commander_version.py +++ b/unit-tests/service/test_min_commander_version.py @@ -48,7 +48,11 @@ def test_missing_header_is_noop(self): self.assertIsNone(check_min_commander_version()) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('18.1.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', '18.1.0', ) def test_accepts_when_current_meets_minimum(self): @@ -60,7 +64,11 @@ def test_accepts_when_current_meets_minimum(self): self.assertIsNone(check_min_commander_version()) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('18.1.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', '18.1.0', ) def test_accepts_case_insensitive_header(self): @@ -72,7 +80,11 @@ def test_accepts_case_insensitive_header(self): self.assertIsNone(check_min_commander_version()) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('18.1.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', '18.1.0', ) def test_accepts_partial_required_version(self): @@ -84,7 +96,11 @@ def test_accepts_partial_required_version(self): self.assertIsNone(check_min_commander_version()) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('17.0.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', '17.0.0', ) def test_rejects_when_current_too_old(self): @@ -99,11 +115,10 @@ def test_rejects_when_current_too_old(self): self.assertIn('17.0.0', body['error']) self.assertIn('18.1.0', body['error']) self.assertIn('Please update Keeper Commander to >= 18.1.0 and retry.', body['error']) - self.assertNotIn('terraform-app-setup', body['error']) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', - '18.1.0', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('18.1.0'), ) def test_rejects_invalid_header(self): with self.app.test_request_context( @@ -117,8 +132,8 @@ def test_rejects_invalid_header(self): self.assertIn('Invalid', body['error']) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', - 'not-a-version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + None, ) def test_rejects_when_running_version_unparseable(self): with self.app.test_request_context( @@ -132,7 +147,11 @@ def test_rejects_when_running_version_unparseable(self): self.assertIn('Unable to determine running Commander version', body['error']) @mock.patch( - 'keepercommander.service.decorators.min_commander_version.commander_version', + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('17.0.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', '17.0.0', ) def test_decorator_blocks_handler(self): diff --git a/unit-tests/service/test_terraform_app_setup.py b/unit-tests/service/test_terraform_app_setup.py index 41cce934f..5e06b0701 100644 --- a/unit-tests/service/test_terraform_app_setup.py +++ b/unit-tests/service/test_terraform_app_setup.py @@ -11,11 +11,14 @@ from unittest import TestCase, mock +from keepercommander.cli import aliases, commands, enterprise_commands, msp_commands +from keepercommander.error import CommandError from keepercommander.service.commands.terraform_app_setup import ( TerraformAppSetupCommand, TerraformSetupConstants, ) from keepercommander.service.docker.models import ServiceConfig +from keepercommander.service.util.exceptions import ValidationError class TestTerraformAppSetupCommand(TestCase): @@ -46,6 +49,29 @@ def test_commands_config_uses_fixed_allowlist(self, mock_runtime_config): TerraformSetupConstants.SERVICE_COMMANDS, params ) + @mock.patch( + 'keepercommander.service.commands.terraform_app_setup.RuntimeServiceConfig' + ) + def test_commands_config_wraps_validation_error(self, mock_runtime_config): + mock_runtime_config.return_value.validate_command_list.side_effect = ValidationError( + 'bad command' + ) + with self.assertRaises(CommandError) as ctx: + TerraformAppSetupCommand()._get_commands_config(mock.Mock()) + self.assertEqual(ctx.exception.command, 'terraform-app-setup') + self.assertIn('bad command', ctx.exception.message) + + @mock.patch.object(TerraformAppSetupCommand, 'run_setup_steps') + @mock.patch.object( + TerraformAppSetupCommand, + '_validate_terraform_commands', + side_effect=CommandError('terraform-app-setup', 'bad allowlist'), + ) + def test_execute_validates_allowlist_before_setup(self, _mock_validate, mock_setup): + with self.assertRaises(CommandError): + TerraformAppSetupCommand().execute(mock.Mock()) + mock_setup.assert_not_called() + @mock.patch.object(TerraformAppSetupCommand, '_get_advanced_security_config') @mock.patch.object(TerraformAppSetupCommand, '_get_cloudflare_config') @mock.patch.object(TerraformAppSetupCommand, '_get_ngrok_config') @@ -95,27 +121,18 @@ def test_service_configuration_always_enables_queue( class TestTerraformSetupConstants(TestCase): - def test_allowlist_includes_core_commands(self): - commands = { - c.strip() - for c in TerraformSetupConstants.SERVICE_COMMANDS.split(',') - if c.strip() - } - for expected in ( - 'this-device', - 'sync-down', - 'enterprise-user', - 'record-add', - 'nsf-share-record', - 'pam', - 'secrets-manager', - ): - self.assertIn(expected, commands) + def test_allowlist_entries_are_registered_commands(self): + known = set(commands) | set(enterprise_commands) | set(msp_commands) | set(aliases) + missing = [ + name for name in TerraformSetupConstants.SERVICE_COMMANDS_LIST + if name not in known + ] + self.assertEqual(missing, []) def test_allowlist_has_no_duplicate_entries(self): - parts = [ - c.strip() - for c in TerraformSetupConstants.SERVICE_COMMANDS.split(',') - if c.strip() - ] + parts = TerraformSetupConstants.SERVICE_COMMANDS_LIST self.assertEqual(len(parts), len(set(parts))) + self.assertEqual( + TerraformSetupConstants.SERVICE_COMMANDS, + ','.join(TerraformSetupConstants.SERVICE_COMMANDS_LIST), + ) From d3c30e2563dbc15340b53811d9ac44f3a9bfd7f0 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 17 Aug 2026 16:21:49 +0530 Subject: [PATCH 4/5] Update container name to include terraform --- .../service/commands/terraform_app_setup.py | 28 ++++++++++++++++++- keepercommander/service/docker/printer.py | 5 ++-- .../service/test_terraform_app_setup.py | 23 +++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py index 74790e9ea..f2db4cb8e 100644 --- a/keepercommander/service/commands/terraform_app_setup.py +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -12,11 +12,18 @@ """Terraform provider Docker service mode setup command.""" import argparse +from dataclasses import asdict from ...commands.base import raise_parse_exception, suppress_exit from ...error import CommandError from ..config.service_config import ServiceConfig as RuntimeServiceConfig -from ..docker import DockerSetupConstants +from ..docker import ( + DockerComposeBuilder, + DockerSetupConstants, + DockerSetupPrinter, + ServiceConfig, + SetupResult, +) from ..util.exceptions import ValidationError from .service_docker_setup import ServiceDockerSetupCommand @@ -27,6 +34,8 @@ class TerraformSetupConstants: DEFAULT_APP_NAME = 'Commander Service Mode - Terraform KSM App' DEFAULT_RECORD_NAME = 'Commander Service Mode Terraform Config' DEFAULT_TIMEOUT = DockerSetupConstants.DEFAULT_TIMEOUT + COMMANDER_SERVICE_NAME = 'commander-terraform' + COMMANDER_CONTAINER_NAME = 'keeper-service-terraform' SERVICE_COMMANDS_LIST = ( 'this-device', 'sync-down', 'switch-to-mc', 'switch-to-msp', @@ -122,3 +131,20 @@ def _get_commands_config(self, params) -> str: def _get_queue_config(self) -> bool: return True + + def generate_docker_compose_yaml(self, setup_result: SetupResult, config: ServiceConfig) -> str: + builder = DockerComposeBuilder( + setup_result, + asdict(config), + commander_service_name=TerraformSetupConstants.COMMANDER_SERVICE_NAME, + commander_container_name=TerraformSetupConstants.COMMANDER_CONTAINER_NAME, + ) + return builder.build() + + def _print_next_steps(self, config: ServiceConfig, config_path: str) -> None: + DockerSetupPrinter.print_common_deployment_steps( + str(config.port), + config_path, + container_name=TerraformSetupConstants.COMMANDER_CONTAINER_NAME, + ) + print() diff --git a/keepercommander/service/docker/printer.py b/keepercommander/service/docker/printer.py index 224e44c30..564e298f7 100644 --- a/keepercommander/service/docker/printer.py +++ b/keepercommander/service/docker/printer.py @@ -61,7 +61,8 @@ def print_phase1_resources(setup_result: SetupResult, indent: str = " ") -> Non print(f"{indent}• KSM Base64 Config: {bcolors.OKGREEN}✓ Generated{bcolors.ENDC}") @staticmethod - def print_common_deployment_steps(port: str, config_path: str = None) -> None: + def print_common_deployment_steps(port: str, config_path: str = None, + container_name: str = 'keeper-service') -> None: """Print common deployment steps (header + steps 1-5)""" DockerSetupPrinter.print_header("Next Steps to Deploy") @@ -82,6 +83,6 @@ def print_common_deployment_steps(port: str, config_path: str = None) -> None: print(f"\n{bcolors.BOLD}Step 5: Check services health{bcolors.ENDC}") print(f" {bcolors.OKGREEN}docker ps{bcolors.ENDC} - View container status") - print(f" {bcolors.OKGREEN}docker logs keeper-service{bcolors.ENDC} - View Commander logs") + print(f" {bcolors.OKGREEN}docker logs {container_name}{bcolors.ENDC} - View Commander logs") print(f" {bcolors.OKGREEN}curl http://localhost:{port}/health{bcolors.ENDC} - Test health endpoint") diff --git a/unit-tests/service/test_terraform_app_setup.py b/unit-tests/service/test_terraform_app_setup.py index 5e06b0701..64ee5e667 100644 --- a/unit-tests/service/test_terraform_app_setup.py +++ b/unit-tests/service/test_terraform_app_setup.py @@ -34,6 +34,29 @@ def test_parser_prog_and_defaults(self): def test_queue_config_always_enabled(self): self.assertTrue(TerraformAppSetupCommand()._get_queue_config()) + def test_compose_uses_terraform_service_and_container_names(self): + setup_result = mock.Mock( + folder_uid='f', folder_name='folder', app_uid='a', app_name='app', + record_uid='r', b64_config='cfg', + ) + config = ServiceConfig( + port=8900, + commands=TerraformSetupConstants.SERVICE_COMMANDS, + queue_enabled=True, + ngrok_enabled=False, + ngrok_auth_token='', + ngrok_custom_domain='', + cloudflare_enabled=False, + cloudflare_tunnel_token='', + cloudflare_custom_domain='', + ) + yaml_content = TerraformAppSetupCommand().generate_docker_compose_yaml( + setup_result, config + ) + self.assertIn('commander-terraform:', yaml_content) + self.assertIn('container_name: keeper-service-terraform', yaml_content) + self.assertNotIn('container_name: keeper-service\n', yaml_content) + @mock.patch( 'keepercommander.service.commands.terraform_app_setup.RuntimeServiceConfig' ) From 1b9d877541822d65aced537e48ff535f28baf4bb Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 18 Aug 2026 12:15:53 +0530 Subject: [PATCH 5/5] Fix for review comments --- .../service/commands/terraform_app_setup.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py index f2db4cb8e..6d4cefe74 100644 --- a/keepercommander/service/commands/terraform_app_setup.py +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -21,7 +21,7 @@ DockerComposeBuilder, DockerSetupConstants, DockerSetupPrinter, - ServiceConfig, + ServiceConfig as DockerServiceConfig, SetupResult, ) from ..util.exceptions import ValidationError @@ -106,11 +106,9 @@ def execute(self, params, **kwargs): kwargs.setdefault('app_name', TerraformSetupConstants.DEFAULT_APP_NAME) kwargs.setdefault('record_name', TerraformSetupConstants.DEFAULT_RECORD_NAME) kwargs.setdefault('timeout', TerraformSetupConstants.DEFAULT_TIMEOUT) - self._validated_commands = self._validate_terraform_commands(params) - try: - return super().execute(params, **kwargs) - finally: - self._validated_commands = None + # Fail closed before any setup; _get_commands_config re-validates locally. + self._validate_terraform_commands(params) + return super().execute(params, **kwargs) def _validate_terraform_commands(self, params) -> str: try: @@ -124,15 +122,12 @@ def _validate_terraform_commands(self, params) -> str: ) def _get_commands_config(self, params) -> str: - cached = getattr(self, '_validated_commands', None) - if cached is not None: - return cached return self._validate_terraform_commands(params) def _get_queue_config(self) -> bool: return True - def generate_docker_compose_yaml(self, setup_result: SetupResult, config: ServiceConfig) -> str: + def generate_docker_compose_yaml(self, setup_result: SetupResult, config: DockerServiceConfig) -> str: builder = DockerComposeBuilder( setup_result, asdict(config), @@ -141,7 +136,7 @@ def generate_docker_compose_yaml(self, setup_result: SetupResult, config: Servic ) return builder.build() - def _print_next_steps(self, config: ServiceConfig, config_path: str) -> None: + def _print_next_steps(self, config: DockerServiceConfig, config_path: str) -> None: DockerSetupPrinter.print_common_deployment_steps( str(config.port), config_path,