Fix kv -mount flag error when mount and secret path are the same (#17679)

* fix mount flag behavior for kv subcommands

* fix mount flag behavior for kv metadata subcommands

* add tests

* add changelog entry

changelog/17679.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+cli: Fix issue preventing kv commands from executing properly when the mount path provided by `-mount` flag and secret key path are the same.
+```

command/kv_delete.go

@@ -117,7 +117,7 @@ func (c *KVDeleteCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -129,12 +129,15 @@ func (c *KVDeleteCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_destroy.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"fmt"
+	"path"
 	"strings"
 
 	"github.com/mitchellh/cli"
@@ -115,7 +116,7 @@ func (c *KVDestroyCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -127,12 +128,15 @@ func (c *KVDestroyCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_get.go

@@ -113,7 +113,7 @@ func (c *KVGetCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath string
@@ -126,12 +126,15 @@ func (c *KVGetCommand) Run(args []string) int {
 	// Parse the paths and grab the KV version
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_metadata_delete.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"fmt"
+	"path"
 	"strings"
 
 	"github.com/mitchellh/cli"
@@ -97,7 +98,7 @@ func (c *KVMetadataDeleteCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -109,12 +110,15 @@ func (c *KVMetadataDeleteCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_metadata_get.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"fmt"
+	"path"
 	"sort"
 	"strconv"
 	"strings"
@@ -99,7 +100,7 @@ func (c *KVMetadataGetCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -111,12 +112,15 @@ func (c *KVMetadataGetCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_metadata_patch.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"path"
 	"strings"
 	"time"
 
@@ -159,7 +160,7 @@ func (c *KVMetadataPatchCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -171,12 +172,15 @@ func (c *KVMetadataPatchCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_metadata_put.go

@@ -3,6 +3,7 @@ package command
 import (
 	"fmt"
 	"io"
+	"path"
 	"strings"
 	"time"
 
@@ -158,7 +159,7 @@ func (c *KVMetadataPutCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -170,12 +171,15 @@ func (c *KVMetadataPutCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_patch.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"strings"
 
 	"github.com/hashicorp/vault/api"
@@ -167,7 +168,7 @@ func (c *KVPatchCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -179,12 +180,15 @@ func (c *KVPatchCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_put.go

@@ -143,7 +143,7 @@ func (c *KVPutCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -155,12 +155,15 @@ func (c *KVPutCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_rollback.go

@@ -3,6 +3,7 @@ package command
 import (
 	"flag"
 	"fmt"
+	"path"
 	"strings"
 
 	"github.com/mitchellh/cli"
@@ -123,7 +124,7 @@ func (c *KVRollbackCommand) Run(args []string) int {
 
 	// If true, we're working with "-mount=secret foo" syntax.
 	// If false, we're using "secret/foo" syntax.
-	mountFlagSyntax := (c.flagMount != "")
+	mountFlagSyntax := c.flagMount != ""
 
 	var (
 		mountPath   string
@@ -135,12 +136,15 @@ func (c *KVRollbackCommand) Run(args []string) int {
 	if mountFlagSyntax {
 		// In this case, this arg is the secret path (e.g. "foo").
 		partialPath = sanitizePath(args[0])
-		mountPath = sanitizePath(c.flagMount)
+		mountPath, v2, err = isKVv2(sanitizePath(c.flagMount), client)
-		_, v2, err = isKVv2(mountPath, client)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 2
 		}
+
+		if v2 {
+			partialPath = path.Join(mountPath, partialPath)
+		}
 	} else {
 		// In this case, this arg is a path-like combination of mountPath/secretPath.
 		// (e.g. "secret/foo")

command/kv_test.go

@@ -129,6 +129,12 @@ func TestKVPutCommand(t *testing.T) {
 			[]string{"Success!"},
 			0,
 		},
+		{
+			"v1_mount_flag_syntax_key_same_as_mount",
+			[]string{"-mount", "secret", "secret", "foo=bar"},
+			[]string{"Success!"},
+			0,
+		},
 		{
 			"v2_single_value",
 			[]string{"kv/write/foo", "foo=bar"},
@@ -153,6 +159,12 @@ func TestKVPutCommand(t *testing.T) {
 			v2ExpectedFields,
 			0,
 		},
+		{
+			"v2_mount_flag_syntax_key_same_as_mount",
+			[]string{"-mount", "kv", "kv", "foo=bar"},
+			v2ExpectedFields,
+			0,
+		},
 		{
 			"v2_single_value_backslash",
 			[]string{"kv/write/foo", "foo=\\"},
@@ -464,6 +476,18 @@ func TestKVGetCommand(t *testing.T) {
 			append(baseV2ExpectedFields, "foo"),
 			0,
 		},
+		{
+			"v1_mount_flag_syntax_key_same_as_mount",
+			[]string{"-mount", "kv", "kv"},
+			append(baseV2ExpectedFields, "foo"),
+			0,
+		},
+		{
+			"v2_mount_flag_syntax_key_same_as_mount",
+			[]string{"-mount", "kv", "kv"},
+			append(baseV2ExpectedFields, "foo"),
+			0,
+		},
 		{
 			"v2_not_found",
 			[]string{"kv/nope/not/once/never"},
@@ -524,6 +548,21 @@ func TestKVGetCommand(t *testing.T) {
 					t.Fatal(err)
 				}
 
+				// create KV entries to test -mount flag where secret key is same as mount path
+				if _, err := client.Logical().Write("secret/secret", map[string]interface{}{
+					"foo": "bar",
+				}); err != nil {
+					t.Fatal(err)
+				}
+
+				if _, err := client.Logical().Write("kv/data/kv", map[string]interface{}{
+					"data": map[string]interface{}{
+						"foo": "bar",
+					},
+				}); err != nil {
+					t.Fatal(err)
+				}
+
 				ui, cmd := testKVGetCommand(t)
 				cmd.client = client
 
@@ -613,6 +652,12 @@ func TestKVMetadataGetCommand(t *testing.T) {
 			expectedTopLevelFields,
 			0,
 		},
+		{
+			"mount_flag_syntax_key_same_as_mount",
+			[]string{"-mount", "kv", "kv"},
+			expectedTopLevelFields,
+			0,
+		},
 	}
 
 	t.Run("validations", func(t *testing.T) {
@@ -643,6 +688,15 @@ func TestKVMetadataGetCommand(t *testing.T) {
 					t.Fatal(err)
 				}
 
+				// create KV entry to test -mount flag where secret key is same as mount path
+				if _, err := client.Logical().Write("kv/data/kv", map[string]interface{}{
+					"data": map[string]interface{}{
+						"foo": "bar",
+					},
+				}); err != nil {
+					t.Fatal(err)
+				}
+
 				ui, cmd := testKVMetadataGetCommand(t)
 				cmd.client = client
 
@@ -973,6 +1027,7 @@ func TestKVPatchCommand_RWMethodSucceeds(t *testing.T) {
 func TestKVPatchCommand_CAS(t *testing.T) {
 	cases := []struct {
 		name       string
+		key        string
 		args       []string
 		expected   string
 		outStrings []string
@@ -980,6 +1035,7 @@ func TestKVPatchCommand_CAS(t *testing.T) {
 	}{
 		{
 			"right version",
+			"foo",
 			[]string{"-cas", "1", "kv/foo", "bar=quux"},
 			"quux",
 			expectedPatchFields(),
@@ -987,6 +1043,7 @@ func TestKVPatchCommand_CAS(t *testing.T) {
 		},
 		{
 			"wrong version",
+			"foo",
 			[]string{"-cas", "2", "kv/foo", "bar=wibble"},
 			"baz",
 			[]string{"check-and-set parameter did not match the current version"},
@@ -994,11 +1051,20 @@ func TestKVPatchCommand_CAS(t *testing.T) {
 		},
 		{
 			"mount_flag_syntax",
+			"foo",
 			[]string{"-mount", "kv", "-cas", "1", "foo", "bar=quux"},
 			"quux",
 			expectedPatchFields(),
 			0,
 		},
+		{
+			"v2_mount_flag_syntax_key_same_as_mount",
+			"kv",
+			[]string{"-mount", "kv", "-cas", "1", "kv", "bar=quux"},
+			"quux",
+			expectedPatchFields(),
+			0,
+		},
 	}
 
 	for _, tc := range cases {
@@ -1030,7 +1096,11 @@ func TestKVPatchCommand_CAS(t *testing.T) {
 
 			kvClient.SetToken(secretAuth.ClientToken)
 
-			_, err = kvClient.Logical().Write("kv/data/foo", map[string]interface{}{"data": map[string]interface{}{"bar": "baz"}})
+			data := map[string]interface{}{
+				"bar": "baz",
+			}
+
+			_, err = kvClient.Logical().Write("kv/data/"+tc.key, map[string]interface{}{"data": data})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1047,7 +1117,7 @@ func TestKVPatchCommand_CAS(t *testing.T) {
 				}
 			}
 
-			secret, err := kvClient.Logical().ReadWithContext(context.Background(), "kv/data/foo")
+			secret, err := kvClient.Logical().ReadWithContext(context.Background(), "kv/data/"+tc.key)
 			if err != nil {
 				t.Fatal(err)
 			}