DynamoDB Nested Values Bug

[MattSurabian]

While doing some deletes of nested paths on a Vault server backed by DynamoDB I noticed some odd behavior where deep nested value deletes could make it seem like the Vault was empty. There are some tests of deep nesting in the code already, but they did not expose this behavior in the DynamoDB backend. Based on my experimentation it seems this behavior occurs when the logical root path only contains "folder" keys and no "value" keys.

This doesn't appear to be a problem for any of the other back ends but it seemed to me like the kind of test that should run on them all. Happy to move this to the DynamoDB test file as a stand alone if folks don't agree.

I'm working on the fix to this issue but wanted to validate the problem with a failed CI run first.

[MattSurabian]

The main issue here seemed to be the interaction between Delete and hasChildren. I updated the logic to account for a couple of main issues:

• If told to delete a path like foo/nested1/nested2/value1 the Delete method would queue the deletion of value1 and then loop through foo/nested1/nested2, foo/nested1, and foo looking for children. An assumption was made that if only 1 child was found that was the "directory" item being queried and it shouldn't be considered a valid child. I'm not at all familiar with the history of the DynamoDB API, but it would appear something has changed since 2015 when that code was written because the referenced base "directory" item no longer gets sent back in the result. The reliance on only a count of children meant that a given path would need at least 2 children for hasChildren to return true. This is problematic because in deep paths there aren't always two children. This was causing overzealous and incorrect empty "folder" cleanup which orphaned deep nested values. (This PR creates a test for this case)
• During "folder" cleanup Delete loops from deepest path to shallowest looking for children: foo/nested1/nested2/nested3, foo/nested1/nested2/, foo/nested1/, foo. Previously, this loop would always run in full. That's a mistake. The point of the loop is to remove empty directory pointer artifacts. If a deeper path contains valid children then it's not possible to cleanup any of the directories that make up a subset of that path string. Doing so could also cause orphaned and unlistable values.

[jefferai]

Hi Matt,

I think the test you wrote makes sense and it's good that it exposed/fixed this behavior in Dynamo. PR looks fine to me!

[kitsirota]

@jefferai do you think we might be able to cut the 0.10.2 release sometime this week?

I just saw all the tickets for this one have been closed. We're dying to get our hands on the latest build, we're stuck right now and not able to delete any records from our DynamoDB backed vaults.

Thanks so much!

[jefferai]

It'll be this week for sure.

[jefferai]

(Hopefully it will be tomorrow)

[scottillogical]

We just ran into a very similar behavior bug to this again

$ safe vault list secret/savannah/som-staging/cheetah/namespaces/dogfood/apps/permissions/
Keys
----
rabbitmq/

$ safe vault  read    secret/savannah/som-staging/cheetah/namespaces/dogfood/apps/permissions/postgresql/default
Key                 Value
---                 -----
refresh_interval    768h
db_name             dogfood__permissions_default

$ safe vault status
Key                      Value
---                      -----
Recovery Seal Type       shamir
Initialized              true
Sealed                   false
Total Recovery Shares    5
Threshold                3
Version                  1.2.2

I plan on following up with Matt on this at some point and we'll open a new issue if we can confirm the behavior on latest vault