Bug: Keeper accepts broken data from RecordEditCommand.execute() which breaks future record access

[erelson]

While evaluating Keeper, I appear to have found a bug resulting from my (mis)use of the python API. The result of this bug is that a folder of records becomes in accessible. Both the browser client and the keeper shell CLI can no longer edit (and sometimes can't access) the records in this folder. Loss of access to records seems like a pretty big deal.

More generally, I'm trying to get recordv3.RecordEditCommand.execute() to do things, but rather than an error message or "nothing happens", it's allowing records to become invalid to some degree, and seems like a 'loss of data access' problem if this was not fake authentication data.

While I'm sure I'm using the API wrong, I wouldn't want developers to be able accidentally break accessibility to our data just because something small is incorrect.

Version: keepercommander==16.9.7

Context on what I'm attempting:

I have a legacy inhouse service with user passwords to hypothetically manage with Keeper. I wrote a python script to rotate these passwords on the legacy service and in Keeper.

Basically:

• Connect to keeper
• Iterate through records (users) in a folder
• Generate new passwords
• Update keeper record for user
• Change password in legacy service

Step 4 has been the part I've struggled with, though I won't cover that journey in depth here. Ultimately, I've been unable to get the python functions to actually update records. I can create new ones; get conflicts if I try to (re)create an existing one. I can use the edit command in keeper shell

Breaking code snippet:

General setup steps:

• Create a folder in Keeper
• Create at least one record in there
• Set up local config.json for Keeper; keeper shell logs in fine. Can also grab the record via python without issue.

Then I run a script similar to the below, which should be able to update fields on the record (just needs to be the password in my usecase):

from keepercommander.commands.recordv3 import RecordEditCommand
from random import randint  # Used to tweak the title to easily see changes made
    
# setup params, sync_down, etc
    
# get folder_uid

for record_id in params.subfolder_record_cache[folder_uid]:
    login = api.get_record(params, record_id)
    
    new_pwd = "thisisgithubissueexample"

    command = RecordEditCommand() 
    print(f"Updating password to: {new_pwd}")
    result = {}
    command.execute(params, record=record_id, return_result=result,
                    data=json.dumps({"password": new_pwd,
                                     "type": "login",
                                     "title": [c.upper() if randint(0,1) else c.lower() for c in login.title]}))

I have tried various permutations of the data= ... parameter, and it always seems to break things. I even tried doing a data=json.dumps(login.to_dictionary()), which ought to be a no-op, I think, and had issues.

Resulting state/errors

• Webpage:
  • Initial breakage: Upon clicking the folder in the Keeper UI, the webpage goes completely white. Browser console shows these errors:

  keeper-vault-a539c60….js:1 TypeError: (e || "").localeCompare is not a function
   at o (keeper-vault-a539c60…4011667.js:1:476566)
   at p (keeper-vault-a539c60…4011667.js:1:478052)
   at Array.sort (<anonymous>)
   at keeper-vault-a539c60…011667.js:1:9866991
   at Object.useMemo (keeper-vault-a539c60…011667.js:1:3381631)
   at t.useMemo (keeper-vault-a539c60…011667.js:1:3665722)
   at keeper-vault-a539c60…011667.js:1:9866576
   at ka (keeper-vault-a539c60…011667.js:1:3375308)
   at Mc (keeper-vault-a539c60…011667.js:1:3432290)
   at Ol (keeper-vault-a539c60…011667.js:1:3421272)
   at ml (keeper-vault-a539c60…011667.js:1:3421200)
   at yl (keeper-vault-a539c60…011667.js:1:3421063)
   at il (keeper-vault-a539c60…011667.js:1:3417870)
   at cl (keeper-vault-a539c60…011667.js:1:3418259)
   at Wo (keeper-vault-a539c60…011667.js:1:3359149)
   at keeper-vault-a539c60…011667.js:1:3415807
  keeper-vault-a539c60….js:1 Uncaught TypeError: (e || "").localeCompare is not a function
   at o (keeper-vault-a539c60…4011667.js:1:476566)
   at p (keeper-vault-a539c60…4011667.js:1:478052)
   at Array.sort (<anonymous>)
   at keeper-vault-a539c60…011667.js:1:9866991
   at Object.useMemo (keeper-vault-a539c60…011667.js:1:3381631)
   at t.useMemo (keeper-vault-a539c60…011667.js:1:3665722)
   at keeper-vault-a539c60…011667.js:1:9866576
   at ka (keeper-vault-a539c60…011667.js:1:3375308)
   at Mc (keeper-vault-a539c60…011667.js:1:3432290)
   at Ol (keeper-vault-a539c60…011667.js:1:3421272)
   at ml (keeper-vault-a539c60…011667.js:1:3421200)
   at yl (keeper-vault-a539c60…011667.js:1:3421063)
   at il (keeper-vault-a539c60…011667.js:1:3417870)
   at cl (keeper-vault-a539c60…011667.js:1:3418259)
   at Wo (keeper-vault-a539c60…011667.js:1:3359149)
   at keeper-vault-a539c60…011667.js:1:3415807
  

  • Upon second repros, can click the record within (new) folder, but webpage goes completely when if I click edit button
  • Upon second repros, can click the record within (new) folder, but can't use roll-back feature. No visible error, but found a 403 + "payment required" in browser console.
• keeper shell Upon running this, I successfully log in, and get the following. The list command similarly breaks:

  Logging in to Keeper Commander
  Successfully authenticated with Password
  Syncing...
  **** Error decrypting record eHSiqLDTNi78MB1EVtaOTA
  **** Error decrypting record btV7fxoVqO2YY7DtE1Y5xw
  Decrypted [3] record(s)
  
  My Vault> list
  An unexpected error occurred: 'list' object has no attribute 'strip'. Type "debug" to toggle verbose error output
  

  Additionally, the list command is entirely broken, even if I cd to a valid folder. So basically this causes account-wide breakage, it seems like.

Lastly, to show the effect in-browser of the above API call approach, an existing record changes from the manually filled out data like:

To a record that is now missing several parts. (It seems, e.g. title is being updated, but type: login isn't working/correct)

[sk-keeper]

Commander provides 2 record add/update command sets

• add and edit
• record-add and record-update https://docs.keeper.io/secrets-manager/commander-cli/command-reference/record-commands#record-add-and-record-update-commands

The former commands are deprecated exactly for that reason. We still keep it in the Commander for compatibility with the previous Commander's versions and sometimes we use them to create broken records intentionally for testing.

The latter commands do not create broken records. You still add record fields that are not supported by all Keeper clients but the record itself is not going to be corrupted.

record-history can be used to rollback a record to the previous version.

[erelson]

Thanks for the quick follow-up @sk-keeper !

You've connected a few dots for me... I had seen that the add/edit commands were deprecated. I failed to realize that I was using the old edit command here.

I was originally playing with the RecordUpdateCommand, as in https://github.com/Keeper-Security/Commander/blob/master/examples/update_password.py#L51 and not having success. Upon seeing that the presumably newer recordv3.py with its RecordEditCommand instead, with similar args, I moved to using that one, assuming latest/greatest. Whoops!

Looking more closely, I'm presently guessing that I want to play with the RecordRecordType next.

[erelson]

I've since had success with the RecordUpdateCommand.

I agree that this bug qualifies as known, and the function is deprecated.

I think it would be reasonable for this function to have an override arg, without which, it would prompt user with a note that this function is deprecated and should not be used. But I defer to the team as of course the risk of this breaking existing Keeper clients is some you're better able to judge.

I do remain confused by commands/recordv3.py module's lack of comments about deprecation or suggested alternates.