Fix undelete for mount paths that are deeper than one level

changelog/19811.txt

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

command/kv_test.go

@@ -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()

command/kv_undelete.go

@@ -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")