From 11da8954e44e9981c2243a7306f5461cdce490ab Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Fri, 18 Sep 2026 10:43:51 +0530 Subject: [PATCH] KC-1458: Restrict app-setup command lists to their own allowlist (#2368) * Restrict app-setup command lists to their own allowlist * Add nsf-move command to terraform-app-setup command list * Handle empty string case * Fix review comments --- .../service/commands/create_service.py | 4 + .../commands/integrations/command_policy.py | 41 ++++++++ .../integrations/integration_setup_base.py | 17 +++- .../commands/integrations/runtime_policy.py | 68 +++++++++++++ .../integrations/sailpoint/command_policy.py | 21 +--- .../integrations/sailpoint_app_setup.py | 6 +- .../service/commands/terraform_app_setup.py | 6 +- unit-tests/service/test_command_policy.py | 59 ++++++++++++ ...integration_setup_base_command_sanitize.py | 70 ++++++++++++++ unit-tests/service/test_runtime_policy.py | 96 +++++++++++++++++++ .../service/test_terraform_app_setup.py | 12 +++ 11 files changed, 379 insertions(+), 21 deletions(-) create mode 100644 keepercommander/service/commands/integrations/command_policy.py create mode 100644 keepercommander/service/commands/integrations/runtime_policy.py create mode 100644 unit-tests/service/test_command_policy.py create mode 100644 unit-tests/service/test_integration_setup_base_command_sanitize.py create mode 100644 unit-tests/service/test_runtime_policy.py diff --git a/keepercommander/service/commands/create_service.py b/keepercommander/service/commands/create_service.py index d035fde91..8ddc5538d 100644 --- a/keepercommander/service/commands/create_service.py +++ b/keepercommander/service/commands/create_service.py @@ -106,6 +106,10 @@ def execute(self, params: KeeperParams, **kwargs) -> None: from .integrations.sailpoint.service import SailPointService SailPointService.maybe_enable(params, args) + # Re-sanitize commands against each integration's own allowlist in case docker-compose.yml is stale/hand-edited. + from .integrations.runtime_policy import apply_runtime_command_policy + apply_runtime_command_policy(args) + from .integrations.vault_metadata import get_existing_api_key, write_service_metadata existing_api_key = ( get_existing_api_key(params, args.update_vault_record) diff --git a/keepercommander/service/commands/integrations/command_policy.py b/keepercommander/service/commands/integrations/command_policy.py new file mode 100644 index 000000000..f9e84d351 --- /dev/null +++ b/keepercommander/service/commands/integrations/command_policy.py @@ -0,0 +1,41 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' str: + """Normalizes `commands` to exactly the allowed set minus banned; cannot narrow below the allowlist, only add missing/drop extra entries.""" + allowed = [c.strip() for c in allowed if c and c.strip()] + allowed_set = {c.lower() for c in allowed} + banned_set = {c.strip().lower() for c in banned} + filtered = [ + cmd for raw in (commands or '').split(',') + if (cmd := raw.strip()) + and (key := cmd.lower()) not in banned_set + and key in allowed_set + ] + # Input order first, then any missing required allowlist entries. + by_key = {cmd.lower(): cmd for cmd in filtered} + for cmd in allowed: + key = cmd.strip().lower() + if key not in banned_set and key not in by_key: + by_key[key] = cmd.strip() + return ','.join(by_key.values()) + + +def default_allowlist(allowed: Iterable[str], banned: Iterable[str] = ()) -> str: + allowed = list(allowed) + return sanitize_commands(','.join(allowed), allowed, banned) diff --git a/keepercommander/service/commands/integrations/integration_setup_base.py b/keepercommander/service/commands/integrations/integration_setup_base.py index 3d26a5169..901b82c09 100644 --- a/keepercommander/service/commands/integrations/integration_setup_base.py +++ b/keepercommander/service/commands/integrations/integration_setup_base.py @@ -30,6 +30,7 @@ ) from .approvals_setup import ApprovalsChannelProfile, collect_approvals_config, is_valid_keeper_uid from .approvals_sync import merge_approvals_custom_fields, run_approvals_sync_down +from .command_policy import sanitize_commands UUID_PATTERN = re.compile( r'^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' @@ -121,6 +122,15 @@ def get_service_commands(self) -> str: commands = f'{commands},{self.get_command_name()}' return commands + def get_banned_commands(self) -> tuple: + """Commands to always strip from this integration's command list; override per integration if needed.""" + return () + + def sanitize_service_commands(self, commands: str) -> str: + """Restrict `commands` to this integration's own get_service_commands() list.""" + allowed = self.get_service_commands().split(',') + return sanitize_commands(commands, allowed, self.get_banned_commands()) + # -- Parser (auto-built from name, cached per subclass) ---------- def get_parser(self): @@ -456,10 +466,13 @@ def _update_docker_compose(self, setup_result: SetupResult, ) try: + cfg = asdict(service_config) + cfg['commands'] = self.sanitize_service_commands(cfg.get('commands') or '') builder = DockerComposeBuilder( - setup_result, asdict(service_config), + setup_result, cfg, commander_service_name=self.get_commander_service_name(), - commander_container_name=self.get_commander_container_name() + commander_container_name=self.get_commander_container_name(), + commander_environment={self.get_record_env_key(): record_uid}, ) yaml_content = builder.add_integration_service( service_name=service_name, diff --git a/keepercommander/service/commands/integrations/runtime_policy.py b/keepercommander/service/commands/integrations/runtime_policy.py new file mode 100644 index 000000000..109e2213b --- /dev/null +++ b/keepercommander/service/commands/integrations/runtime_policy.py @@ -0,0 +1,68 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' None: + if getattr(args, 'commands', None) is None: + return + + sanitizers = _integration_sanitizers() + matched = [env_key for env_key in sanitizers if (os.environ.get(env_key) or '').strip()] + if not matched: + return + if len(matched) > 1: + raise ValidationError( + f'Multiple integration env vars are set ({", ".join(matched)}); ' + f'remove all but one before starting the service.' + ) + + env_key = matched[0] + cleaned = sanitizers[env_key](args.commands) + if cleaned != args.commands: + print( + f'Service Mode ({env_key}): removed commands outside the integration ' + f'allowlist before service-create.\n Was: {args.commands}\n Now: {cleaned}' + ) + args.commands = cleaned + + +def _integration_sanitizers(): + """Maps each integration's env var to its own sanitize_service_commands(); SailPoint is included too as a fallback in case SailPointService.maybe_enable() skipped sanitizing.""" + from ..terraform_app_setup import TerraformSetupConstants + from .command_policy import sanitize_commands + from .gchat_app_setup import GChatAppSetupCommand + from .sailpoint_app_setup import SailPointAppSetupCommand + from .slack_app_setup import SlackAppSetupCommand + from .teams_app_setup import TeamsAppSetupCommand + from ...decorators.min_commander_version import TERRAFORM_DOCKER_ENV + + slack = SlackAppSetupCommand() + teams = TeamsAppSetupCommand() + gchat = GChatAppSetupCommand() + sailpoint = SailPointAppSetupCommand() + + return { + slack.get_record_env_key(): slack.sanitize_service_commands, + teams.get_record_env_key(): teams.sanitize_service_commands, + gchat.get_record_env_key(): gchat.sanitize_service_commands, + sailpoint.get_record_env_key(): sailpoint.sanitize_service_commands, + TERRAFORM_DOCKER_ENV: lambda commands: sanitize_commands( + commands, TerraformSetupConstants.SERVICE_COMMANDS_LIST + ), + } diff --git a/keepercommander/service/commands/integrations/sailpoint/command_policy.py b/keepercommander/service/commands/integrations/sailpoint/command_policy.py index 88ec51451..13afa6b99 100644 --- a/keepercommander/service/commands/integrations/sailpoint/command_policy.py +++ b/keepercommander/service/commands/integrations/sailpoint/command_policy.py @@ -17,6 +17,7 @@ from typing import Any, Optional, Tuple from .....utils import is_email +from .. import command_policy as _shared_policy from .command_parse import SailPointCommandParser from .constants import SAILPOINT_ALLOWED_COMMANDS, SAILPOINT_BANNED_COMMANDS @@ -50,25 +51,13 @@ def sanitize(cls, commands: str) -> str: commands are not dropped when the input list is a partial or older compose allowlist. """ - allowed = {c.strip().lower() for c in SAILPOINT_ALLOWED_COMMANDS} - banned = {c.lower() for c in SAILPOINT_BANNED_COMMANDS} - filtered = [ - cmd for raw in (commands or '').split(',') - if (cmd := raw.strip()) - and (key := cmd.lower()) not in banned - and key in allowed - ] - # Input order first, then any missing required allowlist entries. - by_key = {cmd.lower(): cmd for cmd in filtered} - for cmd in SAILPOINT_ALLOWED_COMMANDS: - key = cmd.lower() - if key not in banned and key not in by_key: - by_key[key] = cmd - return ','.join(by_key.values()) + return _shared_policy.sanitize_commands( + commands, SAILPOINT_ALLOWED_COMMANDS, SAILPOINT_BANNED_COMMANDS + ) @classmethod def default_allowlist(cls) -> str: - return cls.sanitize(','.join(SAILPOINT_ALLOWED_COMMANDS)) + return _shared_policy.default_allowlist(SAILPOINT_ALLOWED_COMMANDS, SAILPOINT_BANNED_COMMANDS) @classmethod def validate_enterprise_user(cls, command: str) -> Optional[str]: diff --git a/keepercommander/service/commands/integrations/sailpoint_app_setup.py b/keepercommander/service/commands/integrations/sailpoint_app_setup.py index 875eeee1d..71c27c6e5 100644 --- a/keepercommander/service/commands/integrations/sailpoint_app_setup.py +++ b/keepercommander/service/commands/integrations/sailpoint_app_setup.py @@ -33,6 +33,7 @@ MIN_POLL_INTERVAL_SECONDS, PENDING_ENTITLEMENTS_FIELD, POLL_INTERVAL_FIELD, + SAILPOINT_BANNED_COMMANDS, SAILPOINT_MARKER_FIELD, SAILPOINT_RECORD_ENV, TRANSFER_TARGET_EMAIL_FIELD, @@ -60,6 +61,9 @@ def get_record_env_key(self) -> str: def get_service_commands(self) -> str: return SailPointCommandPolicy.default_allowlist() + def get_banned_commands(self) -> tuple: + return tuple(SAILPOINT_BANNED_COMMANDS) + def collect_integration_config(self, params, transfer_target_default: str = ''): print(f"\n{bcolors.BOLD}SHARE ENTITLEMENTS:{bcolors.ENDC}") print(f" Control which share entitlements SailPoint may manage via Service Mode") @@ -186,7 +190,7 @@ def _update_docker_compose(self, setup_result, service_config, record_uid, confi try: cfg = asdict(service_config) - cfg['commands'] = SailPointCommandPolicy.sanitize(cfg.get('commands') or '') + cfg['commands'] = self.sanitize_service_commands(cfg.get('commands') or '') builder = DockerComposeBuilder( setup_result, cfg, diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py index b802f0965..85d4b6be6 100644 --- a/keepercommander/service/commands/terraform_app_setup.py +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -26,6 +26,7 @@ ) from ..decorators.min_commander_version import TERRAFORM_DOCKER_ENV from ..util.exceptions import ValidationError +from .integrations.command_policy import sanitize_commands from .service_docker_setup import ServiceDockerSetupCommand @@ -47,7 +48,7 @@ class TerraformSetupConstants: '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', + 'nsf-rm', 'nsf-rndir', 'nsf-share-folder', 'nsf-share-record', 'nsf-ln', 'nsf-move', ) SERVICE_COMMANDS = ','.join(SERVICE_COMMANDS_LIST) @@ -113,7 +114,7 @@ def execute(self, params, **kwargs): def _validate_terraform_commands(self, params) -> str: try: - return RuntimeServiceConfig().validate_command_list( + validated = RuntimeServiceConfig().validate_command_list( TerraformSetupConstants.SERVICE_COMMANDS, params ) except ValidationError as e: @@ -121,6 +122,7 @@ def _validate_terraform_commands(self, params) -> str: self.get_parser().prog, f'Terraform command allowlist validation failed: {e}', ) + return sanitize_commands(validated, TerraformSetupConstants.SERVICE_COMMANDS_LIST) def _get_commands_config(self, params) -> str: return self._validate_terraform_commands(params) diff --git a/unit-tests/service/test_command_policy.py b/unit-tests/service/test_command_policy.py new file mode 100644 index 000000000..a562faeb8 --- /dev/null +++ b/unit-tests/service/test_command_policy.py @@ -0,0 +1,59 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | '