From fac9c57f99bc92cef9c740a45ebedb3f1e938071 Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:06:58 -0500 Subject: [PATCH] Fix sync spinner corrupting console output (garbled KSM config tokens) + validate generated KSM config --- keepercommander/commands/ksm.py | 41 ++++++++++++++++++++++++- keepercommander/display.py | 53 ++++++++++++++++++++++++++------- keepercommander/sync_down.py | 26 ++++++++++------ 3 files changed, 100 insertions(+), 20 deletions(-) diff --git a/keepercommander/commands/ksm.py b/keepercommander/commands/ksm.py index b4ec8ff17..e55463f19 100644 --- a/keepercommander/commands/ksm.py +++ b/keepercommander/commands/ksm.py @@ -10,6 +10,7 @@ # import argparse +import base64 import datetime import hmac import json @@ -1594,6 +1595,8 @@ def init_ksm_config(params, one_time_token, config_init, include_config_dict=Fal if 'KEY_OWNER_PUBLIC_KEY' in ConfigKeys.__members__ and ksm_conf_storage.config.get(ConfigKeys.KEY_OWNER_PUBLIC_KEY): config_dict[ConfigKeys.KEY_OWNER_PUBLIC_KEY.value] = ksm_conf_storage.config.get(ConfigKeys.KEY_OWNER_PUBLIC_KEY) + KSMCommand.validate_ksm_config_dict(config_dict) + converted_config = KSMCommand.convert_config_dict(config_dict, config_init) if include_config_dict: @@ -1604,13 +1607,49 @@ def init_ksm_config(params, one_time_token, config_init, include_config_dict=Fal else: return converted_config + @staticmethod + def validate_ksm_config_dict(config_dict): + """Verify a freshly generated KSM device config is intact. + + The config is handed out as an opaque base64 blob (gateway install, + k8s secret) and a corrupted clientId/privateKey only surfaces much + later as an unusable device, so fail loudly at the source instead. + + Note: if this validation passes but the consumer still receives a + malformed token, the base64 was most likely mangled by the console - + lines overwritten/lost during print (wrapped rows, redraws) or a bad + copy/paste. For comparison capture it losslessly with a redirect: + pam project import ... > out.json + """ + required_keys = ('hostname', 'clientId', 'privateKey', 'serverPublicKeyId', 'appKey') + for key in required_keys: + value = config_dict.get(key) + if not value or not isinstance(value, str): + raise Exception(f'Generated KSM config is invalid: "{key}" is missing or empty. ' + 'Please remove the client device and try again.') + for key in ('clientId', 'privateKey', 'appKey'): + try: + decoded = base64.b64decode(config_dict[key], validate=True) + except Exception: + raise Exception(f'Generated KSM config is invalid: "{key}" is not valid base64. ' + 'Please remove the client device and try again.') + if key == 'clientId' and len(decoded) != 64: # HMAC-SHA512 digest + raise Exception(f'Generated KSM config is invalid: "clientId" decodes to ' + f'{len(decoded)} bytes, expected 64. ' + 'Please remove the client device and try again.') + @staticmethod def convert_config_dict(config_dict, conversion_type='json'): config = json.dumps(config_dict) if conversion_type in ['b64', 'k8s']: - config = json_to_base64(config) + encoded = json_to_base64(config) + # the encoded blob must round-trip to the exact JSON it was built + # from - catches any corruption before the config is handed out + if base64.b64decode(encoded).decode('utf-8') != config: + raise Exception('KSM config base64 encoding failed the integrity check') + config = encoded if conversion_type == 'k8s': config = "\n" \ diff --git a/keepercommander/display.py b/keepercommander/display.py index 8e23f0e15..7958c0214 100644 --- a/keepercommander/display.py +++ b/keepercommander/display.py @@ -254,7 +254,17 @@ def print_record(params, record_uid): class Spinner: - """Animated spinner for long-running operations.""" + """Animated spinner for long-running operations. + + WARNING: every frame starts with '\\r' and overwrites the current console + row with the frame, message and padding spaces. A spinner that is still + (or again) ticking while other code prints can therefore erase chunks of + large multi-row output - especially long single-line blobs like base64 + KSM config tokens (`pam project import`/`pam gateway new` access_token), + silently corrupting what the user copies. Callers MUST guarantee stop() + via try/finally, and any change here must keep frames out of stopped + spinners and out of redirected/captured output. + """ # Claude-style spinner frames FRAMES = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'] @@ -272,26 +282,49 @@ def _animate(self): message = self.message or '' visible_len = len(message) + 2 # frame + space + message pad = max(0, self._last_visible_len - visible_len) - sys.stdout.write(f'\r{Fore.CYAN}{frame}{Fore.RESET} {message}' + (' ' * pad)) - sys.stdout.flush() + # Re-check right before the write: a stale tick firing after + # stop() has returned (join timed out on a blocked console write) + # would '\r'-overwrite output printed in the meantime - erasing a + # row of large output such as a printed KSM config token. + # Skipping costs only one cosmetic frame. + if not self.running: + break + # Frames go to stderr (codebase convention for '\r' progress, see + # sox/aram/record_totp): on stdout they land inside redirected or + # captured command output, e.g. corrupting the base64 config in + # `pam project import ... > out.json`. + sys.stderr.write(f'\r{Fore.CYAN}{frame}{Fore.RESET} {message}' + (' ' * pad)) + sys.stderr.flush() self._last_visible_len = visible_len + pad idx += 1 time.sleep(0.08) - # Clear the line when done - clear_len = max(self._last_visible_len, len(self.message or '') + 2) - sys.stdout.write('\r' + ' ' * clear_len + '\r') - sys.stdout.flush() - self._last_visible_len = 0 def start(self): + # Spin only on a real console; in a redirected/captured stream the + # frames cannot animate and would pile up as '\r' noise in the data. + try: + if not sys.stderr.isatty(): + return + except Exception: + return self.running = True self.thread = threading.Thread(target=self._animate, daemon=True) self.thread.start() def stop(self): self.running = False - if self.thread: - self.thread.join(timeout=0.5) + if not self.thread: + return + self.thread.join(timeout=0.5) + self.thread = None + # Clear the spinner line from the calling thread after the animator + # has exited; clearing from the animator raced with output printed + # right after stop() and could blank part of it (erased lines in + # large output, e.g. ksm config tokens) with the padding spaces. + clear_len = max(self._last_visible_len, len(self.message or '') + 2) + sys.stderr.write('\r' + ' ' * clear_len + '\r') + sys.stderr.flush() + self._last_visible_len = 0 def post_login_summary(record_count=0, breachwatch_count=0, show_tips=True): diff --git a/keepercommander/sync_down.py b/keepercommander/sync_down.py index c72015451..85f7fca28 100644 --- a/keepercommander/sync_down.py +++ b/keepercommander/sync_down.py @@ -27,14 +27,26 @@ def sync_down(params, record_types=False): # type: (KeeperParams, bool) -> None """Sync full or partial data down to the client""" - params.sync_data = False - token = params.sync_down_token - - # Use spinner animation for full sync (only in interactive mode, not batch/automation) + # Use spinner animation for full sync (only in interactive mode, not batch/automation). + # WARNING: stop() MUST be guaranteed via finally. A leaked spinner thread keeps + # '\r'-overwriting the current console row for the rest of the session, erasing + # lines of any large output printed later - notably base64 KSM config tokens + # (`pam project import` / `pam gateway new` access_token), which then reach the + # user silently corrupted. spinner = None - if not token and not params.batch_mode: + if not params.sync_down_token and not params.batch_mode: spinner = Spinner('Syncing...') spinner.start() + try: + _sync_down_impl(params, record_types) + finally: + if spinner: + spinner.stop() + + +def _sync_down_impl(params, record_types=False): # type: (KeeperParams, bool) -> None + params.sync_data = False + token = params.sync_down_token for record in params.record_cache.values(): if 'shares' in record: @@ -1041,10 +1053,6 @@ def convert_user_folder_shared_folder(ufsf): type_id += rt.scope * 1000000 params.record_type_cache[type_id] = rt.content - # Stop spinner if running - if spinner: - spinner.stop() - if full_sync: convert_keys.change_key_types(params)