Fix undelete for mount paths that are deeper than one level (#19811)

* Fix for undelete for mount paths

* Pulled in a fix from kv_delete.go that adds partialPath to
  the mountPath to support paths that are multiple levels deep.
* Added undelete tests to validate that KV secrets can be successfully
  undeleted when mounted at a multi-level mount path.
* Added changelog txt file

* Update changelog to specify KV impact

---------

Co-authored-by: Violet Hynes <violet.hynes@hashicorp.com>
This commit is contained in:
Nick Downs 2023-10-31 06:08:12 -07:00 committed by GitHub
parent 21a07110e1
commit b9b72ec321
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 169 additions and 8 deletions

3
changelog/19811.txt Normal file
View file

@ -0,0 +1,3 @@
```release-note:bug
cli/kv: Undelete now properly handles KV-V2 mount paths that are more than one layer deep.
```

View file

@ -1522,6 +1522,155 @@ func TestPadEqualSigns(t *testing.T) {
})
}
}
func testKVUndeleteCommand(tb testing.TB) (*cli.MockUi, *KVUndeleteCommand) {
tb.Helper()
ui := cli.NewMockUi()
return ui, &KVUndeleteCommand{
BaseCommand: &BaseCommand{
UI: ui,
},
}
}
func TestKVUndeleteCommand(t *testing.T) {
t.Parallel()
cases := []struct {
name string
args []string
outStrings []string
code int
}{
{
"not_enough_args",
[]string{},
[]string{"Not enough arguments"},
1,
},
{
"too_many_args",
[]string{"foo", "bar"},
[]string{"Too many arguments"},
1,
},
{
"no_versions",
[]string{"-mount", "kv", "/read/foo"},
[]string{"No versions provided"},
1,
},
{
"v2_mount_flag_syntax",
[]string{"-versions", "1", "-mount", "kv", "read/foo"},
[]string{"Success! Data written to: kv/undelete/read/foo"},
0,
},
{
"v2_mount_flag_syntax_complex_1",
[]string{"-versions", "1", "-mount", "secrets/testapp", "test"},
[]string{"Success! Data written to: secrets/testapp/undelete/test"},
0,
},
{
"v2_mount_flag_syntax_complex_2",
[]string{"-versions", "1", "-mount", "secrets/x/testapp", "test"},
[]string{"Success! Data written to: secrets/x/testapp/undelete/test"},
0,
},
}
t.Run("validations", func(t *testing.T) {
t.Parallel()
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
client, closer := testVaultServer(t)
defer closer()
if err := client.Sys().Mount("kv/", &api.MountInput{
Type: "kv-v2",
}); err != nil {
t.Fatal(err)
}
if err := client.Sys().Mount("secrets/testapp", &api.MountInput{
Type: "kv-v2",
}); err != nil {
t.Fatal(err)
}
// Additional layer of mount path
if err := client.Sys().Mount("secrets/x/testapp", &api.MountInput{
Type: "kv-v2",
}); err != nil {
t.Fatal(err)
}
// Give time for the upgrade code to run/finish
time.Sleep(time.Second)
if _, err := client.Logical().Write("kv/data/read/foo", map[string]interface{}{
"data": map[string]interface{}{
"foo": "bar",
},
}); err != nil {
t.Fatal(err)
}
// Delete the entry so we can undelete it
if _, err := client.Logical().Delete("kv/data/read/foo"); err != nil {
t.Fatal(err)
}
if _, err := client.Logical().Write("secrets/testapp/data/test", map[string]interface{}{
"data": map[string]interface{}{
"complex": "yes",
},
}); err != nil {
t.Fatal(err)
}
if _, err := client.Logical().Write("secrets/x/testapp/data/test", map[string]interface{}{
"data": map[string]interface{}{
"complex": "yes",
},
}); err != nil {
t.Fatal(err)
}
// Delete the entry so we can undelete it
if _, err := client.Logical().Delete("secrets/x/testapp/data/test"); err != nil {
t.Fatal(err)
}
// Delete the entry so we can undelete it
if _, err := client.Logical().Delete("secrets/testapp/data/test"); err != nil {
t.Fatal(err)
}
ui, cmd := testKVUndeleteCommand(t)
cmd.client = client
code := cmd.Run(tc.args)
if code != tc.code {
t.Errorf("expected %d to be %d", code, tc.code)
}
combined := ui.OutputWriter.String() + ui.ErrorWriter.String()
for _, str := range tc.outStrings {
if !strings.Contains(combined, str) {
t.Errorf("expected %q to contain %q", combined, str)
}
}
})
}
})
}
func createTokenForPolicy(t *testing.T, client *api.Client, policy string) (*api.SecretAuth, error) {
t.Helper()

View file

@ -5,6 +5,7 @@ package command
import (
"fmt"
"path"
"strings"
"github.com/mitchellh/cli"
@ -35,12 +36,12 @@ Usage: vault kv undelete [options] KEY
This restores the data, allowing it to be returned on get requests.
To undelete version 3 of key "foo":
$ vault kv undelete -mount=secret -versions=3 foo
The deprecated path-like syntax can also be used, but this should be avoided,
as the fact that it is not actually the full API path to
the secret (secret/data/foo) can cause confusion:
The deprecated path-like syntax can also be used, but this should be avoided,
as the fact that it is not actually the full API path to
the secret (secret/data/foo) can cause confusion:
$ vault kv undelete -versions=3 secret/foo
@ -67,10 +68,10 @@ func (c *KVUndeleteCommand) Flags() *FlagSets {
Name: "mount",
Target: &c.flagMount,
Default: "", // no default, because the handling of the next arg is determined by whether this flag has a value
Usage: `Specifies the path where the KV backend is mounted. If specified,
the next argument will be interpreted as the secret path. If this flag is
not specified, the next argument will be interpreted as the combined mount
path and secret path, with /data/ automatically appended between KV
Usage: `Specifies the path where the KV backend is mounted. If specified,
the next argument will be interpreted as the secret path. If this flag is
not specified, the next argument will be interpreted as the combined mount
path and secret path, with /data/ automatically appended between KV
v2 secrets.`,
})
@ -134,6 +135,14 @@ func (c *KVUndeleteCommand) Run(args []string) int {
c.UI.Error(err.Error())
return 2
}
if v2 {
// Without this join, mountPaths that are deeper
// than the root path E.G. secrets/myapp will get
// pruned down to myapp/undelete/<secret> which
// is incorrect.
// This technique was lifted from kv_delete.go.
partialPath = path.Join(mountPath, partialPath)
}
} else {
// In this case, this arg is a path-like combination of mountPath/secretPath.
// (e.g. "secret/foo")