Fix: sync spinner corrupting large console output (garbled KSM config tokens)

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" \

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):

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)