From f56dcbae42011b047190d42e56206b93e33316e3 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 7 Jul 2026 17:59:54 +0530 Subject: [PATCH 1/2] Fix docker startup multiple syncs --- docker-entrypoint.sh | 41 ++++++++----------- keepercommander/commands/utils.py | 2 + .../service/commands/handle_service.py | 6 ++- unit-tests/service/test_service_manager.py | 4 ++ unit-tests/test_command_utils.py | 4 ++ 5 files changed, 32 insertions(+), 25 deletions(-) diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 19b265193..91ffad491 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -159,38 +159,31 @@ process_ksm_config() { # DEVICE SETUP AND AUTHENTICATION FUNCTIONS # ============================================================================= -# Setup device registration and persistent login +# Setup device registration and persistent login. +# All three steps run in a single Commander process so login + vault sync +# only happens once instead of three times. setup_device() { local user="$1" local password="$2" local server="$3" - - # Step 1: Register device - log "Registering device..." - if ! python3 keeper.py --user "${user}" --password "${password}" \ - --server "${server}" this-device register; then - log "ERROR: Device registration failed" - exit 1 - fi - - # Step 2: Enable persistent login - log "Enabling persistent login..." - if ! python3 keeper.py --user "${user}" --password "${password}" \ - --server "${server}" this-device persistent-login on; then - log "ERROR: Persistent login setup failed" - exit 1 - fi - # Step 3: Set timeout - log "Setting device logout timeout to 30 Days..." + log "Running device setup (register, persistent login, timeout)..." + local setup_script + setup_script=$(mktemp /tmp/keeper_setup_XXXXXX.cmd) + cat > "${setup_script}" < /dev/null; then - log "ERROR: Timeout setup failed" + --server "${server}" "${setup_script}"; then + log "ERROR: Device setup failed" + rm -f "${setup_script}" exit 1 fi - - log "Device Logout Timeout set successfully" + + rm -f "${setup_script}" log "Device setup completed successfully" } diff --git a/keepercommander/commands/utils.py b/keepercommander/commands/utils.py index cf5f2b336..75b737906 100644 --- a/keepercommander/commands/utils.py +++ b/keepercommander/commands/utils.py @@ -521,6 +521,8 @@ def execute(self, params, **kwargs): class ThisDeviceCommand(Command): + skip_sync_on_auth = True + def get_parser(self): return this_device_parser diff --git a/keepercommander/service/commands/handle_service.py b/keepercommander/service/commands/handle_service.py index d18398498..9cf1c7a97 100644 --- a/keepercommander/service/commands/handle_service.py +++ b/keepercommander/service/commands/handle_service.py @@ -43,7 +43,11 @@ class ServiceStatus(Command): def get_parser(self): parser = argparse.ArgumentParser(prog='service-status', parents=[report_output_parser], description='Displays if the Commander API service is running or stopped') return parser - + + def is_authorised(self): + # Local process/PID check only — no vault access required. + return False + def execute(self, params: KeeperParams, **kwargs) -> str: status = ServiceManager.get_status() print(f"Current status: {status}") \ No newline at end of file diff --git a/unit-tests/service/test_service_manager.py b/unit-tests/service/test_service_manager.py index f1798ac30..63379aba9 100644 --- a/unit-tests/service/test_service_manager.py +++ b/unit-tests/service/test_service_manager.py @@ -115,6 +115,10 @@ def test_service_status_when_not_running(self): status_cmd.execute(self.params) mock_print.assert_called_with("Current status: No Commander Service is running currently") + def test_service_status_does_not_require_auth(self): + """service-status reads local process info only — must not require login or sync.""" + self.assertFalse(ServiceStatus().is_authorised()) + def test_process_info_save_load(self): """Test ProcessInfo save and load operations""" test_pid = 12345 diff --git a/unit-tests/test_command_utils.py b/unit-tests/test_command_utils.py index a06fe846e..69677347f 100644 --- a/unit-tests/test_command_utils.py +++ b/unit-tests/test_command_utils.py @@ -64,6 +64,10 @@ def test_logout(self): cmd.execute(params) self.assertIsNone(params.session_token) + def test_this_device_skips_sync_on_auth(self): + """Device setup commands must not trigger a full vault sync on auto-login.""" + self.assertTrue(utils.ThisDeviceCommand.skip_sync_on_auth) + def test_enterprise_invite(self): params = get_connected_params() params.enforcements = { From 583263b7cc7285fef408d692286a14dd722d5c8a Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Wed, 8 Jul 2026 11:35:35 +0530 Subject: [PATCH 2/2] Remove sync from 'service-status' and keep it in 'this-device' --- keepercommander/commands/utils.py | 2 -- keepercommander/service/commands/handle_service.py | 7 +++---- keepercommander/service/docker/printer.py | 4 +++- unit-tests/service/test_service_manager.py | 6 +++--- unit-tests/test_command_utils.py | 4 ---- 5 files changed, 9 insertions(+), 14 deletions(-) diff --git a/keepercommander/commands/utils.py b/keepercommander/commands/utils.py index 75b737906..cf5f2b336 100644 --- a/keepercommander/commands/utils.py +++ b/keepercommander/commands/utils.py @@ -521,8 +521,6 @@ def execute(self, params, **kwargs): class ThisDeviceCommand(Command): - skip_sync_on_auth = True - def get_parser(self): return this_device_parser diff --git a/keepercommander/service/commands/handle_service.py b/keepercommander/service/commands/handle_service.py index 9cf1c7a97..8051ea92f 100644 --- a/keepercommander/service/commands/handle_service.py +++ b/keepercommander/service/commands/handle_service.py @@ -39,15 +39,14 @@ def execute(self, params: KeeperParams, **kwargs) -> None: class ServiceStatus(Command): """Command to get service status.""" + + skip_sync_on_auth = True + @debug_decorator def get_parser(self): parser = argparse.ArgumentParser(prog='service-status', parents=[report_output_parser], description='Displays if the Commander API service is running or stopped') return parser - def is_authorised(self): - # Local process/PID check only — no vault access required. - return False - def execute(self, params: KeeperParams, **kwargs) -> str: status = ServiceManager.get_status() print(f"Current status: {status}") \ No newline at end of file diff --git a/keepercommander/service/docker/printer.py b/keepercommander/service/docker/printer.py index 28d593729..af1b66dfd 100644 --- a/keepercommander/service/docker/printer.py +++ b/keepercommander/service/docker/printer.py @@ -13,6 +13,8 @@ Output formatting utilities for Docker setup commands. """ +import shlex + from ...display import bcolors from .models import SetupResult @@ -68,7 +70,7 @@ def print_common_deployment_steps(port: str, config_path: str = None) -> None: config_file = config_path if config_path else '~/.keeper/config.json' print(f"\n{bcolors.BOLD}Step 2: Delete the local config.json file{bcolors.ENDC}") - print(f" {bcolors.OKGREEN}rm {config_file}{bcolors.ENDC}") + print(f" {bcolors.OKGREEN}rm {shlex.quote(config_file)}{bcolors.ENDC}") print(f" Why? Prevents device token conflicts - Docker will download its own config.") print(f"\n{bcolors.BOLD}Step 3: Review docker-compose.yml{bcolors.ENDC}") diff --git a/unit-tests/service/test_service_manager.py b/unit-tests/service/test_service_manager.py index 63379aba9..45ac3925b 100644 --- a/unit-tests/service/test_service_manager.py +++ b/unit-tests/service/test_service_manager.py @@ -115,9 +115,9 @@ def test_service_status_when_not_running(self): status_cmd.execute(self.params) mock_print.assert_called_with("Current status: No Commander Service is running currently") - def test_service_status_does_not_require_auth(self): - """service-status reads local process info only — must not require login or sync.""" - self.assertFalse(ServiceStatus().is_authorised()) + def test_service_status_skips_sync_on_auth(self): + """service-status still requires login but must not trigger a full vault sync.""" + self.assertTrue(ServiceStatus.skip_sync_on_auth) def test_process_info_save_load(self): """Test ProcessInfo save and load operations""" diff --git a/unit-tests/test_command_utils.py b/unit-tests/test_command_utils.py index 69677347f..a06fe846e 100644 --- a/unit-tests/test_command_utils.py +++ b/unit-tests/test_command_utils.py @@ -64,10 +64,6 @@ def test_logout(self): cmd.execute(params) self.assertIsNone(params.session_token) - def test_this_device_skips_sync_on_auth(self): - """Device setup commands must not trigger a full vault sync on auto-login.""" - self.assertTrue(utils.ThisDeviceCommand.skip_sync_on_auth) - def test_enterprise_invite(self): params = get_connected_params() params.enforcements = {