runtimevar/hashivault: reject secret paths that escape the mount

runtimevar/hashivault/hashivault.go

@@ -42,6 +42,7 @@ import (
 	"os"
 	"path"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -271,11 +272,15 @@ func newWatcher(client *api.Client, secretPath string, decoder *runtimevar.Decod
 		mount = "secret"
 	}
 
-	var fullPath string
+	var prefix string
 	if engineVersion == 2 {
-		fullPath = path.Join(mount, "data", secretPath)
+		prefix = path.Join(mount, "data")
 	} else {
-		fullPath = path.Join(mount, secretPath)
+		prefix = mount
+	}
+	fullPath, err := vaultPath(prefix, secretPath)
+	if err != nil {
+		return nil, err
 	}
 
 	return &watcher{
@@ -288,6 +293,20 @@ func newWatcher(client *api.Client, secretPath string, decoder *runtimevar.Decod
 	}, nil
 }
 
+// vaultPath joins prefix and secretPath into a Vault API path, and verifies
+// that the result is actually inside prefix. path.Join cleans ".." segments,
+// so a secretPath such as "../../sys/health" would otherwise silently make
+// the request escape the intended mount -- something Vault's own
+// path-prefix-scoped ACL policies can't defend against, since as far as
+// Vault is concerned it's simply the path the client asked for.
+func vaultPath(prefix, secretPath string) (string, error) {
+	p := path.Join(prefix, secretPath)
+	if p != prefix && !strings.HasPrefix(p, prefix+"/") {
+		return "", fmt.Errorf("hashivault: secret path %q escapes the %q mount", secretPath, prefix)
+	}
+	return p, nil
+}
+
 type state struct {
 	val        any
 	raw        *api.Secret

runtimevar/hashivault/hashivault_test.go

@@ -262,6 +262,50 @@ func TestEngineVersionPaths(t *testing.T) {
 	}
 }
 
+func TestSecretPathEscape(t *testing.T) {
+	ctx := context.Background()
+	client, err := Dial(ctx, &Config{
+		Token: "fake",
+		APIConfig: api.Config{
+			Address: "http://localhost:8200",
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tests := []struct {
+		SecretPath string
+		WantPath   string // empty means an error is expected
+	}{
+		{"tenant-a/config", "secret/data/tenant-a/config"},
+		{"tenant-a/../tenant-b/config", "secret/data/tenant-b/config"}, // stays within the mount, allowed
+		{"../../sys/health", ""},
+		{"a/../../../auth/token/create", ""},
+		{"../secret/data/other-tenant", ""},
+	}
+
+	for _, test := range tests {
+		w, err := newWatcher(client, test.SecretPath, runtimevar.StringDecoder, nil)
+		if test.WantPath == "" {
+			if err == nil {
+				t.Errorf("SecretPath=%q: expected an error rejecting the escape, got none (path=%q)",
+					test.SecretPath, w.(*watcher).path)
+			}
+			continue
+		}
+		if err != nil {
+			t.Errorf("SecretPath=%q: newWatcher failed: %v", test.SecretPath, err)
+			continue
+		}
+		watcher := w.(*watcher)
+		if watcher.path != test.WantPath {
+			t.Errorf("SecretPath=%q: got path %q, want %q", test.SecretPath, watcher.path, test.WantPath)
+		}
+		w.Close()
+	}
+}
+
 func TestInvalidEngineVersion(t *testing.T) {
 	ctx := context.Background()
 	client, err := Dial(ctx, &Config{