From 4a5921cb1639c67491bd63337345cf65ad1de8ae Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Wed, 7 Aug 2024 09:26:18 -0400
Subject: [PATCH] acl: disallow leading `/` on variable paths (#23757)

The path for a Variable never begins with a leading `/`, because it's stripped
off in the API before it ever gets to the state store. The CLI and UI allow the
leading `/` for convenience, but this can be misleading when it comes to writing
ACL policies. An ACL policy with a path starting with a leading `/` will never
match.

Update the ACL policy parser so that we prevent an incorrect variable path in
the policy.

Fixes: https://github.com/hashicorp/nomad/issues/23730
---
 .changelog/23757.txt                                |  3 +++
 acl/policy.go                                       |  6 ++++++
 acl/policy_test.go                                  | 13 +++++++++++++
 .../docs/other-specifications/acl-policy.mdx        |  4 +++-
 4 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 .changelog/23757.txt

diff --git a/.changelog/23757.txt b/.changelog/23757.txt
new file mode 100644
index 000000000..660874c18
--- /dev/null
+++ b/.changelog/23757.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+acl: Submitting a policy with a leading `/` in a variable path will now return an error to prevent improperly working policies.
+```
diff --git a/acl/policy.go b/acl/policy.go
index a290316d8..c4fe9e4d6 100644
--- a/acl/policy.go
+++ b/acl/policy.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
+	"strings"
 
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
@@ -395,6 +396,11 @@ func Parse(rules string) (*Policy, error) {
 				if pathPolicy.PathSpec == "" {
 					return nil, fmt.Errorf("Invalid missing variable path in namespace %s", ns.Name)
 				}
+				if strings.HasPrefix(pathPolicy.PathSpec, "/") {
+					return nil, fmt.Errorf(
+						"Invalid variable path %q in namespace %s: cannot start with a leading '/'`",
+						pathPolicy.PathSpec, ns.Name)
+				}
 				for _, cap := range pathPolicy.Capabilities {
 					if !isPathCapabilityValid(cap) {
 						return nil, fmt.Errorf(
diff --git a/acl/policy_test.go b/acl/policy_test.go
index 6d36a3ce9..117b82ba3 100644
--- a/acl/policy_test.go
+++ b/acl/policy_test.go
@@ -494,6 +494,19 @@ func TestParse(t *testing.T) {
 			"Invalid variable policy: no variable paths in namespace dev",
 			nil,
 		},
+		{
+			`
+			namespace "dev" {
+			  variables {
+                path "/nomad/job" {
+			      capabilities = ["read", "write"]
+                }
+			  }
+			}
+			`,
+			"Invalid variable path \"/nomad/job\" in namespace dev: cannot start with a leading '/'",
+			nil,
+		},
 		{
 			`
 			namespace "dev" {
diff --git a/website/content/docs/other-specifications/acl-policy.mdx b/website/content/docs/other-specifications/acl-policy.mdx
index d23a53cb9..0b0f1b7d5 100644
--- a/website/content/docs/other-specifications/acl-policy.mdx
+++ b/website/content/docs/other-specifications/acl-policy.mdx
@@ -199,7 +199,9 @@ variables block per namespace rule.
 
 A `variables` block includes one or more `path` blocks. Each `path` block is
 labeled with the path it applies to. You may use wildcard globs (`"*"`) in the
-path label, to apply the block to multiple paths in the namespace.
+path label, to apply the block to multiple paths in the namespace. Note that
+variable paths never start with a leading `/`, so Nomad will return an error if
+you submit a policy that has such a path.
 
 Each path has a list of `capabilities`. The available capabilities for Variables
 are as follows: