KC-1403: Fix NSF record update "This object no longer exists" after PAM tunnel/connection edit

keepercommander/commands/pam/vault_target.py

@@ -581,6 +581,9 @@ def update_pam_record(params, record, command='pam', force_nsf=False):
         sync_down_preserving_nsf_keys(params)
     else:
         record_management.update_record(params, record)
+        # Defer vault refresh so a second classic edit in the same session does
+        # not send a stale record_cache revision (no immediate sync_down here).
+        params.sync_data = True
 
 
 def execute_record_add_in_folder(params, args, folder_uid, command='pam'):

keepercommander/commands/tunnel_and_connections.py

@@ -45,7 +45,7 @@
     stop_tunnel_process,
     unregister_tunnel,
 )
-from .. import api, vault, record_management
+from .. import api, vault
 from ..display import bcolors
 from ..error import CommandError
 from ..params import LAST_RECORD_UID
@@ -2983,8 +2983,7 @@ def _get_effective_protocol():
                         logging.debug(f'security is already {target_sec} on record={record_uid}')
 
             if dirty:
-                record_management.update_record(params, record)
-                api.sync_down(params)
+                update_pam_record(params, record, command='pam connection edit')
 
                 traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed')
                 if not traffic_encryption_key:
@@ -4055,8 +4054,7 @@ def update_connection_choice(field_name, value):
             update_connection_choice('sessionPersistence', session_persistence)
 
         if dirty:
-            record_management.update_record(params, record)
-            api.sync_down(params)
+            update_pam_record(params, record, command='pam rbi edit')
 
             traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed')
             if not traffic_encryption_key:
@@ -4235,7 +4233,7 @@ def execute(self, params, **kwargs):
                     pam_settings = vault.TypedField.new_field('pamSettings', "", "")
                     record.fields.append(pam_settings)
 
-                record_management.update_record(params, record)
+                update_pam_record(params, record, command='pam-split')
                 params.sync_data = True
 
                 print(f"{bcolors.WARNING}Record {record_uid} has no data to split and "
@@ -4291,7 +4289,7 @@ def execute(self, params, **kwargs):
             pam_settings = vault.TypedField.new_field('pamSettings', "", "")
             record.fields.append(pam_settings)
 
-        record_management.update_record(params, record)
+        update_pam_record(params, record, command='pam-split')
         params.sync_data = True
 
         if pam_config_uid:

keepercommander/nested_share_folder/__init__.py

@@ -19,7 +19,8 @@
     ],
     'common': [
         'get_folder_key', 'get_record_key', 'get_user_public_key',
-        'get_record_from_cache', 'parse_sharing_status', 'get_record_key_type',
+        'get_record_from_cache', 'get_record_revision', 'patch_record_revision',
+        'parse_sharing_status', 'get_record_key_type',
         'encrypt_record_key_for_folder', 'encrypt_for_recipient',
         'handle_share_invite', 'resolve_user_uid_bytes',
         'load_user_public_key', 'parse_folder_access_result',

keepercommander/nested_share_folder/common.py

@@ -56,6 +56,32 @@ def get_record_from_cache(params, record_uid: str) -> Optional[dict]:
     return None
 
 
+def get_record_revision(params, record_uid: str, default: int = 0) -> int:
+    """Return the highest known revision across NSF and classic caches.
+
+    NSF metadata and classic vault sync can diverge; always prefer the
+    freshest revision when sending optimistic-concurrency updates.
+    """
+    revisions = []
+    for attr in ('nested_share_records', 'record_cache'):
+        cache = getattr(params, attr, None) or {}
+        if record_uid in cache:
+            rev = cache[record_uid].get('revision')
+            if rev is not None:
+                revisions.append(rev)
+    return max(revisions) if revisions else default
+
+
+def patch_record_revision(params, record_uid: str, revision: int) -> None:
+    """Write *revision* into both NSF and classic caches when the UID is present."""
+    if not revision:
+        return
+    for attr in ('nested_share_records', 'record_cache'):
+        cache = getattr(params, attr, None)
+        if cache and record_uid in cache:
+            cache[record_uid]['revision'] = revision
+
+
 def get_record_key_type(params, record_uid: str) -> Optional[int]:
     """Return the record key type if available (legacy AES-CBC vs AES-GCM)."""
     meta = getattr(params, 'meta_data_cache', {}).get(record_uid)

keepercommander/nested_share_folder/record_api.py

@@ -15,6 +15,7 @@
 
 from .common import (
     get_folder_key, get_record_key, get_record_from_cache,
+    get_record_revision, patch_record_revision,
     get_user_public_key, encrypt_for_recipient, handle_share_invite,
     parse_sharing_status,
 )
@@ -173,62 +174,116 @@ def _load_existing_record_data(params, record_uid, rec=None):
     return None
 
 
+def _apply_record_update_overrides(data, title=None, record_type=None, fields=None, notes=None):
+    """Apply optional title/type/fields/notes overrides onto a record data dict."""
+    if title is not None:
+        data['title'] = title
+    if record_type is not None:
+        data['type'] = record_type
+    if fields is not None:
+        by_type = {}
+        for ef in data.get('fields', []):
+            by_type.setdefault(ef.get('type'), []).append(ef)
+        for ft, fv in fields.items():
+            fv = fv if isinstance(fv, list) else [fv]
+            if ft in by_type and by_type[ft]:
+                by_type[ft][0]['value'] = fv
+            else:
+                data.setdefault('fields', []).append({'type': ft, 'value': fv})
+    if notes is not None:
+        data['notes'] = notes
+    return data
+
+
+def _sync_down_for_nsf_update(params):
+    """Sync while preserving NSF folder keys when possible."""
+    try:
+        from ..commands.pam_import.nsf_helpers import sync_down_preserving_nsf_keys
+        sync_down_preserving_nsf_keys(params)
+    except Exception:
+        from .. import sync_down as sync_down_mod
+        sync_down_mod.sync_down(params)
+
+
 def update_record_v3(params, record_uid, data=None, title=None,
                      record_type=None, fields=None, notes=None,
                      non_shared_data=None, revision=None):
+    """Update an NSF/classic-cached record via vault/records/v3/update.
+
+    On ``RS_OUT_OF_SYNC`` with no explicit *revision*, syncs once and retries.
+    When *data* was omitted, the retry rebuilds payload from refreshed caches
+    so a concurrent content edit is not silently overwritten. When the caller
+    supplied *data* (e.g. ``update_pam_record``), the retry re-sends that
+    payload at the freshest revision (force / last-write-wins for that call).
+    """
     rec = get_record_from_cache(params, record_uid)
     if not rec:
-        from .. import sync_down
-        sync_down.sync_down(params)
+        _sync_down_for_nsf_update(params)
         rec = get_record_from_cache(params, record_uid)
     if not rec:
         raise ValueError(f"Record {record_uid} not found")
 
     rk = rec.get('record_key_unencrypted') or get_record_key(params, record_uid)
+    caller_supplied_data = data is not None
 
     if data is None:
         existing = _load_existing_record_data(params, record_uid, rec)
         data = existing.copy() if existing else {'fields': []}
-        if title is not None:
-            data['title'] = title
-        if record_type is not None:
-            data['type'] = record_type
-        if fields is not None:
-            by_type = {}
-            for ef in data.get('fields', []):
-                by_type.setdefault(ef.get('type'), []).append(ef)
-            for ft, fv in fields.items():
-                fv = fv if isinstance(fv, list) else [fv]
-                if ft in by_type and by_type[ft]:
-                    by_type[ft][0]['value'] = fv
-                else:
-                    data.setdefault('fields', []).append({'type': ft, 'value': fv})
-        if notes is not None:
-            data['notes'] = notes
-
-    ru = record_pb2.RecordUpdate()
-    ru.record_uid = utils.base64_url_decode(record_uid)
-    ru.client_modified_time = utils.current_milli_time()
-    ru.revision = revision if revision is not None else rec.get('revision', 0)
-
-    dj = pad_aes_gcm(json.dumps(data))
-    db = dj.encode() if isinstance(dj, str) else dj
-    ru.data = crypto.encrypt_aes_v2(db, rk)
-
-    if non_shared_data:
-        nsj = pad_aes_gcm(json.dumps(non_shared_data))
-        nsb = nsj.encode() if isinstance(nsj, str) else nsj
-        ru.non_shared_data = crypto.encrypt_aes_v2(nsb, rk)
-
+        _apply_record_update_overrides(data, title, record_type, fields, notes)
+
+    def _build_update(payload, rev):
+        ru = record_pb2.RecordUpdate()
+        ru.record_uid = utils.base64_url_decode(record_uid)
+        ru.client_modified_time = utils.current_milli_time()
+        # Prefer the highest known revision: NSF metadata can lag classic sync_down.
+        ru.revision = (rev if rev is not None
+                       else get_record_revision(params, record_uid, rec.get('revision', 0)))
+        dj = pad_aes_gcm(json.dumps(payload))
+        db = dj.encode() if isinstance(dj, str) else dj
+        ru.data = crypto.encrypt_aes_v2(db, rk)
+        if non_shared_data:
+            nsj = pad_aes_gcm(json.dumps(non_shared_data))
+            nsb = nsj.encode() if isinstance(nsj, str) else nsj
+            ru.non_shared_data = crypto.encrypt_aes_v2(nsb, rk)
+        return ru
+
+    ru = _build_update(data, revision)
     response = record_update_v3(params, [ru])
     if response.records:
         r = response.records[0]
+        # After PAM/classic edits, sync can leave NSF metadata lagging. Refresh
+        # once and retry with the freshest known revision.
+        if r.status == record_pb2.RS_OUT_OF_SYNC and revision is None:
+            _sync_down_for_nsf_update(params)
+            rec = get_record_from_cache(params, record_uid) or rec
+            rk = rec.get('record_key_unencrypted') or rk
+            if caller_supplied_data:
+                logging.warning(
+                    'NSF record update retry for %s after RS_OUT_OF_SYNC may overwrite '
+                    'a concurrent content edit (caller-supplied data).',
+                    record_uid,
+                )
+                retry_data = data
+            else:
+                existing = _load_existing_record_data(params, record_uid, rec)
+                retry_data = existing.copy() if existing else {'fields': []}
+                _apply_record_update_overrides(
+                    retry_data, title, record_type, fields, notes)
+            ru = _build_update(retry_data, None)
+            response = record_update_v3(params, [ru])
+            if not response.records:
+                raise KeeperApiError('no_results', 'No results from record update')
+            r = response.records[0]
+        success = r.status == record_pb2.RS_SUCCESS
+        new_revision = getattr(response, 'revision', 0)
+        if success:
+            patch_record_revision(params, record_uid, new_revision)
         return {
             'record_uid': record_uid,
             'status': record_pb2.RecordModifyResult.Name(r.status),
             'message': r.message,
-            'success': r.status == record_pb2.RS_SUCCESS,
-            'revision': getattr(response, 'revision', 0),
+            'success': success,
+            'revision': new_revision,
         }
     raise KeeperApiError('no_results', 'No results from record update')
 

keepercommander/nested_share_folder/sync.py

@@ -386,9 +386,14 @@ def _process_records(params, records):
     """Store DriveRecord metadata (no encrypted content)."""
     for record in records:
         record_uid = utils.base64_url_encode(record.recordUid)
+        existing = params.nested_share_records.get(record_uid) or {}
+        # Classic vault updates can bump revision before NSF drive metadata
+        # catches up. Never allow a lagging DriveRecord to downgrade the cache.
+        incoming_rev = record.revision or 0
+        existing_rev = existing.get('revision', 0) or 0
         record_obj = {
             'record_uid': record_uid,
-            'revision': record.revision,
+            'revision': max(incoming_rev, existing_rev),
             'version': record.version,
             'shared': record.shared if record.shared else False,
             'client_modified_time': record.clientModifiedTime if record.clientModifiedTime else 0,
@@ -397,6 +402,9 @@ def _process_records(params, records):
             record_obj['file_size'] = record.fileSize
         if record.thumbnailSize:
             record_obj['thumbnail_size'] = record.thumbnailSize
+        # Preserve decrypted key material across metadata refreshes.
+        if 'record_key_unencrypted' in existing:
+            record_obj['record_key_unencrypted'] = existing['record_key_unencrypted']
         params.nested_share_records[record_uid] = record_obj
 
 
@@ -1136,9 +1144,32 @@ def _reconstruct_nested_share_folder_entities(params):
         if 'data_json' not in rd_obj:
             continue
 
+        classic = params.record_cache.get(record_uid) or {}
+        classic_rev = classic.get('revision', 0) or 0
+        nsf_rev = record_obj.get('revision', 0) or 0
+        # Prefer the freshest revision across classic sync and NSF metadata.
+        revision = max(classic_rev, nsf_rev)
+        if revision != nsf_rev:
+            record_obj['revision'] = revision
+
+        # Classic response.records payloads use encrypted 'data' and are not tagged
+        # source=nested_share_folder. Prefer that copy when it is at least as fresh
+        # so lagging keeperDriveData cannot roll content/revision backwards after PAM.
+        classic_from_vault = (
+            'data' in classic and classic.get('source') != 'nested_share_folder'
+        )
+        if classic_from_vault and classic_rev >= nsf_rev:
+            if 'record_key_unencrypted' not in classic:
+                classic['record_key_unencrypted'] = record_obj['record_key_unencrypted']
+            # Still backfill meta/owner caches — NSF is often the only source on
+            # fresh login, and this branch is taken whenever classic sync returns
+            # the record (e.g. after PAM edits).
+            _backfill_nsf_record_access_caches(params, record_uid, record_obj, rd_obj)
+            continue
+
         record_entry = {
             'record_uid': record_uid,
-            'revision': record_obj.get('revision', 0),
+            'revision': revision,
             'version': record_obj.get('version', 0),
             'shared': record_obj.get('shared', False),
             'record_key_unencrypted': record_obj['record_key_unencrypted'],
@@ -1149,24 +1180,28 @@ def _reconstruct_nested_share_folder_entities(params):
         }
 
         params.record_cache[record_uid] = record_entry
+        _backfill_nsf_record_access_caches(params, record_uid, record_obj, rd_obj)
 
-        if record_uid not in params.meta_data_cache:
-            meta_data = {
-                'record_uid': record_uid,
-                'record_key_unencrypted': record_obj['record_key_unencrypted'],
-                'can_share': True,
-                'can_edit': True,
-            }
-            if 'user_account_uid' in rd_obj:
-                meta_data['owner_account_uid'] = rd_obj['user_account_uid']
-                if rd_obj['user_account_uid'] in params.user_cache:
-                    meta_data['owner_username'] = params.user_cache[rd_obj['user_account_uid']]
-            params.meta_data_cache[record_uid] = meta_data
-
-        if record_uid not in params.record_owner_cache:
-            if 'user_account_uid' in rd_obj:
-                is_owner = (rd_obj['user_account_uid'] == utils.base64_url_encode(params.account_uid_bytes))
-                params.record_owner_cache[record_uid] = RecordOwner(
-                    is_owner,
-                    rd_obj['user_account_uid']
-                )
+
+def _backfill_nsf_record_access_caches(params, record_uid, record_obj, rd_obj):
+    """Populate meta_data_cache / record_owner_cache from NSF data when missing."""
+    if record_uid not in params.meta_data_cache:
+        meta_data = {
+            'record_uid': record_uid,
+            'record_key_unencrypted': record_obj['record_key_unencrypted'],
+            'can_share': True,
+            'can_edit': True,
+        }
+        if 'user_account_uid' in rd_obj:
+            meta_data['owner_account_uid'] = rd_obj['user_account_uid']
+            if rd_obj['user_account_uid'] in params.user_cache:
+                meta_data['owner_username'] = params.user_cache[rd_obj['user_account_uid']]
+        params.meta_data_cache[record_uid] = meta_data
+
+    if record_uid not in params.record_owner_cache:
+        if 'user_account_uid' in rd_obj:
+            is_owner = (rd_obj['user_account_uid'] == utils.base64_url_encode(params.account_uid_bytes))
+            params.record_owner_cache[record_uid] = RecordOwner(
+                is_owner,
+                rd_obj['user_account_uid']
+            )

keepercommander/sync_down.py

@@ -287,7 +287,24 @@ def convert_record(r):
 
             for r in response.records:
                 record = convert_record(r)
-                params.record_cache[record['record_uid']] = record
+                record_uid = record['record_uid']
+                params.record_cache[record_uid] = record
+                # Classic vault updates bump revision without NSF keeperDriveData.
+                # Keep nested_share_records.revision aligned so NSF updates do not
+                # send a stale revision (RS_OUT_OF_SYNC / "This object no longer exists").
+                nsf_records = getattr(params, 'nested_share_records', None)
+                if nsf_records and record_uid in nsf_records:
+                    # Direct assignment is intentional: classic response.records is
+                    # vault-authoritative for this UID after a classic update. NSF
+                    # _process_records uses max() because keeperDriveData can lag;
+                    # here the classic stream is the fresher source of truth.
+                    nsf_rec = nsf_records[record_uid]
+                    nsf_rec['revision'] = record['revision']
+                    nsf_rec['version'] = record['version']
+                    # shared/client_modified_time follow the classic payload for the
+                    # same reason; NSF-only sharing state is refreshed from drive data.
+                    nsf_rec['shared'] = record['shared']
+                    nsf_rec['client_modified_time'] = record['client_modified_time']
 
         if len(response.nonSharedData) > 0:
             for nsd in response.nonSharedData: