Merge branch 'main' into fix_logrus_CVE

.changelog/21871.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+proxycfg: fix a bug where peered upstreams watches are canceled even when another target needs it.
+```

.changelog/21908.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Resolved issue where hcl would allow duplicates of the same key in acl policy configuration.
+```

.changelog/21950.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Removed ability to use bexpr to filter results without ACL read on endpoint
+```

.changelog/21951.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Update `github.com/golang-jwt/jwt/v4` to v4.5.1 to address [GHSA-29wx-vh33-7x7r](https://github.com/golang-jwt/jwt/security/advisories/GHSA-29wx-vh33-7x7r).
+```

.github/CODEOWNERS

@@ -1,3 +1,5 @@
+* @hashicorp/consul-selfmanage-maintainers
+
 # Techical Writer Review
 
 /website/content/docs/ @hashicorp/consul-docs
@@ -6,8 +8,8 @@
 
 
 # release configuration
-/.release/                              @hashicorp/release-engineering @hashicorp/github-consul-core
+/.release/                              @hashicorp/team-selfmanaged-releng @hashicorp/consul-selfmanage-maintainers
-/.github/workflows/build.yml            @hashicorp/release-engineering @hashicorp/github-consul-core
+/.github/workflows/build.yml            @hashicorp/team-selfmanaged-releng @hashicorp/consul-selfmanage-maintainers
 
 
 # Staff Engineer Review (protocol buffer definitions)

acl/acl.go

@@ -22,6 +22,9 @@ type Config struct {
 	// WildcardName is the string that represents a request to authorize a wildcard permission
 	WildcardName string
 
+	//by default errors, but in certain instances we want to make sure to maintain backwards compatabilty
+	WarnOnDuplicateKey bool
+
 	// embedded enterprise configuration
 	EnterpriseConfig
 }

acl/policy.go

@@ -310,8 +310,8 @@ func (pr *PolicyRules) Validate(conf *Config) error {
 	return nil
 }
 
-func parse(rules string, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
+func parse(rules string, warnOnDuplicateKey bool, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
-	p, err := decodeRules(rules, conf, meta)
+	p, err := decodeRules(rules, warnOnDuplicateKey, conf, meta)
 	if err != nil {
 		return nil, err
 	}
@@ -338,7 +338,11 @@ func NewPolicyFromSource(rules string, conf *Config, meta *EnterprisePolicyMeta)
 
 	var policy *Policy
 	var err error
-	policy, err = parse(rules, conf, meta)
+	warnOnDuplicateKey := false
+	if conf != nil {
+		warnOnDuplicateKey = conf.WarnOnDuplicateKey
+	}
+	policy, err = parse(rules, warnOnDuplicateKey, conf, meta)
 	return policy, err
 }
 

acl/policy_ce.go

@@ -7,8 +7,9 @@ package acl
 
 import (
 	"fmt"
-
+	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/hcl"
+	"strings"
 )
 
 // EnterprisePolicyMeta stub
@@ -30,12 +31,28 @@ func (r *EnterprisePolicyRules) Validate(*Config) error {
 	return nil
 }
 
-func decodeRules(rules string, _ *Config, _ *EnterprisePolicyMeta) (*Policy, error) {
+func decodeRules(rules string, warnOnDuplicateKey bool, _ *Config, _ *EnterprisePolicyMeta) (*Policy, error) {
 	p := &Policy{}
 
-	if err := hcl.Decode(p, rules); err != nil {
+	err := hcl.DecodeErrorOnDuplicates(p, rules)
+
+	if errIsDuplicateKey(err) && warnOnDuplicateKey {
+		//because the snapshot saves the unparsed rules we have to assume some snapshots exist that shouldn't fail, but
+		// have duplicates
+		if err := hcl.Decode(p, rules); err != nil {
+			hclog.Default().Warn("Warning- Duplicate key in ACL Policy ignored", "errorMessage", err.Error())
+			return nil, fmt.Errorf("Failed to parse ACL rules: %v", err)
+		}
+	} else if err != nil {
 		return nil, fmt.Errorf("Failed to parse ACL rules: %v", err)
 	}
 
 	return p, nil
 }
+
+func errIsDuplicateKey(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(err.Error(), "was already set. Each argument can only be defined once")
+}

acl/policy_test.go

@@ -342,6 +342,12 @@ func TestPolicySourceParse(t *testing.T) {
 			RulesJSON: `{ "acl": "list" }`, // there is no list policy but this helps to exercise another check in isPolicyValid
 			Err:       "Invalid acl policy",
 		},
+		{
+			Name: "Bad Policy - Duplicate ACL Key",
+			Rules: `acl="read"
+					acl="write"`,
+			Err: "Failed to parse ACL rules: The argument \"acl\" at",
+		},
 		{
 			Name:      "Bad Policy - Agent",
 			Rules:     `agent "foo" { policy = "nope" }`,

agent/agent_endpoint.go

@@ -380,16 +380,14 @@ func (s *HTTPHandlers) AgentServices(resp http.ResponseWriter, req *http.Request
 		return nil, err
 	}
 
-	raw, err := filter.Execute(agentSvcs)
+	// Note: we filter the results with ACLs *before* applying the user-supplied
-	if err != nil {
+	// bexpr filter to ensure that the user can only run expressions on data that
-		return nil, err
+	// they have access to.  This is a security measure to prevent users from
-	}
+	// running arbitrary expressions on data they don't have access to.
-	agentSvcs = raw.(map[string]*api.AgentService)
+	// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
-
+	// that results they don't have access to have been removed.  If they were
-	// Note: we filter the results with ACLs *after* applying the user-supplied
+	// also allowed to run the bexpr filter on the data, they could potentially
-	// bexpr filter, to ensure total (and the filter-by-acls header we set below)
+	// infer the specific attributes of data they don't have access to.
-	// do not include results that would be filtered out even if the user did have
-	// permission.
 	total := len(agentSvcs)
 	if err := s.agent.filterServicesWithAuthorizer(authz, agentSvcs); err != nil {
 		return nil, err
@@ -407,6 +405,12 @@ func (s *HTTPHandlers) AgentServices(resp http.ResponseWriter, req *http.Request
 		setResultsFilteredByACLs(resp, total != len(agentSvcs))
 	}
 
+	raw, err := filter.Execute(agentSvcs)
+	if err != nil {
+		return nil, err
+	}
+	agentSvcs = raw.(map[string]*api.AgentService)
+
 	return agentSvcs, nil
 }
 
@@ -540,16 +544,14 @@ func (s *HTTPHandlers) AgentChecks(resp http.ResponseWriter, req *http.Request)
 		}
 	}
 
-	raw, err := filter.Execute(agentChecks)
+	// Note: we filter the results with ACLs *before* applying the user-supplied
-	if err != nil {
+	// bexpr filter to ensure that the user can only run expressions on data that
-		return nil, err
+	// they have access to.  This is a security measure to prevent users from
-	}
+	// running arbitrary expressions on data they don't have access to.
-	agentChecks = raw.(map[types.CheckID]*structs.HealthCheck)
+	// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
-
+	// that results they don't have access to have been removed.  If they were
-	// Note: we filter the results with ACLs *after* applying the user-supplied
+	// also allowed to run the bexpr filter on the data, they could potentially
-	// bexpr filter, to ensure total (and the filter-by-acls header we set below)
+	// infer the specific attributes of data they don't have access to.
-	// do not include results that would be filtered out even if the user did have
-	// permission.
 	total := len(agentChecks)
 	if err := s.agent.filterChecksWithAuthorizer(authz, agentChecks); err != nil {
 		return nil, err
@@ -567,6 +569,12 @@ func (s *HTTPHandlers) AgentChecks(resp http.ResponseWriter, req *http.Request)
 		setResultsFilteredByACLs(resp, total != len(agentChecks))
 	}
 
+	raw, err := filter.Execute(agentChecks)
+	if err != nil {
+		return nil, err
+	}
+	agentChecks = raw.(map[types.CheckID]*structs.HealthCheck)
+
 	return agentChecks, nil
 }
 
@@ -623,21 +631,14 @@ func (s *HTTPHandlers) AgentMembers(resp http.ResponseWriter, req *http.Request)
 		}
 	}
 
-	// filter the members by parsed filter expression
+	// Note: we filter the results with ACLs *before* applying the user-supplied
-	var filterExpression string
+	// bexpr filter to ensure that the user can only run expressions on data that
-	s.parseFilter(req, &filterExpression)
+	// they have access to.  This is a security measure to prevent users from
-	if filterExpression != "" {
+	// running arbitrary expressions on data they don't have access to.
-		filter, err := bexpr.CreateFilter(filterExpression, nil, members)
+	// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
-		if err != nil {
+	// that results they don't have access to have been removed.  If they were
-			return nil, err
+	// also allowed to run the bexpr filter on the data, they could potentially
-		}
+	// infer the specific attributes of data they don't have access to.
-		raw, err := filter.Execute(members)
-		if err != nil {
-			return nil, err
-		}
-		members = raw.([]serf.Member)
-	}
-
 	total := len(members)
 	if err := s.agent.filterMembers(token, &members); err != nil {
 		return nil, err
@@ -655,6 +656,21 @@ func (s *HTTPHandlers) AgentMembers(resp http.ResponseWriter, req *http.Request)
 		setResultsFilteredByACLs(resp, total != len(members))
 	}
 
+	// filter the members by parsed filter expression
+	var filterExpression string
+	s.parseFilter(req, &filterExpression)
+	if filterExpression != "" {
+		filter, err := bexpr.CreateFilter(filterExpression, nil, members)
+		if err != nil {
+			return nil, err
+		}
+		raw, err := filter.Execute(members)
+		if err != nil {
+			return nil, err
+		}
+		members = raw.([]serf.Member)
+	}
+
 	return members, nil
 }
 

agent/agent_endpoint_test.go

@@ -433,6 +433,60 @@ func TestAgent_Services_ACLFilter(t *testing.T) {
 		require.Len(t, val, 2)
 		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
 	})
+
+	// ensure ACL filtering occurs before bexpr filtering.
+	const bexprMatchingUserTokenPermissions = "Service matches `web.*`"
+	const bexprNotMatchingUserTokenPermissions = "Service matches `api.*`"
+
+	tokenWithWebRead := testCreateToken(t, a, `
+			service "web" {
+				policy = "read"
+			}
+		`)
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithWebRead)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		var val map[string]*api.AgentService
+		err := dec.Decode(&val)
+		if err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 1)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithWebRead)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		var val map[string]*api.AgentService
+		err := dec.Decode(&val)
+		if err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/services?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		var val map[string]*api.AgentService
+		err := dec.Decode(&val)
+		if err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
 }
 
 func TestAgent_Service(t *testing.T) {
@@ -1432,6 +1486,57 @@ func TestAgent_Checks_ACLFilter(t *testing.T) {
 		require.Len(t, val, 2)
 		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
 	})
+
+	// ensure ACL filtering occurs before bexpr filtering.
+	const bexprMatchingUserTokenPermissions = "ServiceName matches `web.*`"
+	const bexprNotMatchingUserTokenPermissions = "ServiceName matches `api.*`"
+
+	tokenWithWebRead := testCreateToken(t, a, `
+			service "web" {
+				policy = "read"
+			}
+		`)
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithWebRead)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make(map[types.CheckID]*structs.HealthCheck)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 1)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithWebRead)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make(map[types.CheckID]*structs.HealthCheck)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/checks?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make(map[types.CheckID]*structs.HealthCheck)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
 }
 
 func TestAgent_Self(t *testing.T) {
@@ -2110,6 +2215,57 @@ func TestAgent_Members_ACLFilter(t *testing.T) {
 		require.Len(t, val, 2)
 		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
 	})
+
+	// ensure ACL filtering occurs before bexpr filtering.
+	bexprMatchingUserTokenPermissions := fmt.Sprintf("Name matches `%s.*`", b.Config.NodeName)
+	bexprNotMatchingUserTokenPermissions := fmt.Sprintf("Name matches `%s.*`", a.Config.NodeName)
+
+	tokenWithReadOnMemberB := testCreateToken(t, a, fmt.Sprintf(`
+			node "%s" {
+				policy = "read"
+			}
+		`, b.Config.NodeName))
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithReadOnMemberB)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make([]serf.Member, 0)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 1)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		req.Header.Add("X-Consul-Token", tokenWithReadOnMemberB)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make([]serf.Member, 0)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.NotEmpty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "/v1/agent/members?filter="+url.QueryEscape(bexprNotMatchingUserTokenPermissions), nil)
+		resp := httptest.NewRecorder()
+		a.srv.h.ServeHTTP(resp, req)
+		dec := json.NewDecoder(resp.Body)
+		val := make([]serf.Member, 0)
+		if err := dec.Decode(&val); err != nil {
+			t.Fatalf("Err: %v", err)
+		}
+		require.Len(t, val, 0)
+		require.Empty(t, resp.Header().Get("X-Consul-Results-Filtered-By-ACLs"))
+	})
 }
 
 func TestAgent_Join(t *testing.T) {

agent/catalog_endpoint_test.go

@@ -122,7 +122,7 @@ func TestCatalogDeregister(t *testing.T) {
 	a := NewTestAgent(t, "")
 	defer a.Shutdown()
 
-	// Register node
+	// Deregister node
 	args := &structs.DeregisterRequest{Node: "foo"}
 	req, _ := http.NewRequest("PUT", "/v1/catalog/deregister", jsonReader(args))
 	obj, err := a.srv.CatalogDeregister(nil, req)

agent/consul/acl_endpoint_test.go

@@ -2169,6 +2169,22 @@ func TestACLEndpoint_PolicySet(t *testing.T) {
 		require.Error(t, err)
 	})
 
+	t.Run("Key Dup", func(t *testing.T) {
+		req := structs.ACLPolicySetRequest{
+			Datacenter: "dc1",
+			Policy: structs.ACLPolicy{
+				Description: "foobar",
+				Name:        "baz2",
+				Rules:       "service \"\" { policy = \"read\" policy = \"write\" }",
+			},
+			WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken},
+		}
+		resp := structs.ACLPolicy{}
+
+		err := aclEp.PolicySet(&req, &resp)
+		require.Error(t, err)
+	})
+
 	t.Run("Update it", func(t *testing.T) {
 		req := structs.ACLPolicySetRequest{
 			Datacenter: "dc1",

agent/consul/catalog_endpoint.go

@@ -533,18 +533,23 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
 				return nil
 			}
 
-			raw, err := filter.Execute(reply.Nodes)
+			// Note: we filter the results with ACLs *before* applying the user-supplied
-			if err != nil {
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
-			reply.Nodes = raw.(structs.Nodes)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
+			raw, err := filter.Execute(reply.Nodes)
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			if err != nil {
-			// results that would be filtered out even if the user did have permission.
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
+			reply.Nodes = raw.(structs.Nodes)
 
 			return c.srv.sortNodesByDistanceFrom(args.Source, reply.Nodes)
 		})
@@ -607,14 +612,25 @@ func (c *Catalog) ListServices(args *structs.DCSpecificRequest, reply *structs.I
 				return nil
 			}
 
-			raw, err := filter.Execute(serviceNodes)
+			// need to temporarily create an IndexedServiceNode so that the ACL filter can be applied
+			// to the service nodes and then re-use those same node to run the filter expression.
+			idxServiceNodeReply := &structs.IndexedServiceNodes{
+				ServiceNodes: serviceNodes,
+				QueryMeta:    reply.QueryMeta,
+			}
+
+			// enforce ACLs
+			c.srv.filterACLWithAuthorizer(authz, idxServiceNodeReply)
+
+			// run the filter expression
+			raw, err := filter.Execute(idxServiceNodeReply.ServiceNodes)
 			if err != nil {
 				return err
 			}
 
+			// convert the result back to the original type
 			reply.Services = servicesTagsByName(raw.(structs.ServiceNodes))
-
+			reply.QueryMeta = idxServiceNodeReply.QueryMeta
-			c.srv.filterACLWithAuthorizer(authz, reply)
 
 			return nil
 		})
@@ -813,6 +829,18 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 				reply.ServiceNodes = filtered
 			}
 
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			// This is safe to do even when the filter is nil - its just a no-op then
 			raw, err := filter.Execute(reply.ServiceNodes)
 			if err != nil {
@@ -820,13 +848,6 @@ func (c *Catalog) ServiceNodes(args *structs.ServiceSpecificRequest, reply *stru
 			}
 			reply.ServiceNodes = raw.(structs.ServiceNodes)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
-			// results that would be filtered out even if the user did have permission.
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			return c.srv.sortNodesByDistanceFrom(args.Source, reply.ServiceNodes)
 		})
 
@@ -904,6 +925,18 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
 			}
 			reply.Index, reply.NodeServices = index, services
 
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := c.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			if reply.NodeServices != nil {
 				raw, err := filter.Execute(reply.NodeServices.Services)
 				if err != nil {
@@ -912,13 +945,6 @@ func (c *Catalog) NodeServices(args *structs.NodeSpecificRequest, reply *structs
 				reply.NodeServices.Services = raw.(map[string]*structs.NodeService)
 			}
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
-			// results that would be filtered out even if the user did have permission.
-			if err := c.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			return nil
 		})
 }
@@ -1009,21 +1035,26 @@ func (c *Catalog) NodeServiceList(args *structs.NodeSpecificRequest, reply *stru
 
 			if mergedServices != nil {
 				reply.NodeServices = *mergedServices
-
-				raw, err := filter.Execute(reply.NodeServices.Services)
-				if err != nil {
-					return err
-				}
-				reply.NodeServices.Services = raw.([]*structs.NodeService)
 			}
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
+			// Note: we filter the results with ACLs *before* applying the user-supplied
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			// bexpr filter to ensure that the user can only run expressions on data that
-			// results that would be filtered out even if the user did have permission.
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
 			if err := c.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
 
+			raw, err := filter.Execute(reply.NodeServices.Services)
+			if err != nil {
+				return err
+			}
+			reply.NodeServices.Services = raw.([]*structs.NodeService)
+
 			return nil
 		})
 }

agent/consul/catalog_endpoint_test.go

@@ -984,26 +984,27 @@ func TestCatalog_RPC_Filter(t *testing.T) {
 		require.Equal(t, "baz", out.Nodes[0].Node)
 	})
 
-	t.Run("ServiceNodes", func(t *testing.T) {
+	t.Run("ListServices", func(t *testing.T) {
 		args := structs.ServiceSpecificRequest{
 			Datacenter:   "dc1",
 			ServiceName:  "redis",
 			QueryOptions: structs.QueryOptions{Filter: "ServiceMeta.version == 1"},
 		}
 
-		out := new(structs.IndexedServiceNodes)
+		out := new(structs.IndexedServices)
-		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out))
-		require.Len(t, out.ServiceNodes, 2)
+		require.Len(t, out.Services, 2)
-		require.Condition(t, func() bool {
+		require.Len(t, out.Services["redis"], 1)
-			return (out.ServiceNodes[0].Node == "foo" && out.ServiceNodes[1].Node == "bar") ||
+		require.Len(t, out.Services["web"], 2)
-				(out.ServiceNodes[0].Node == "bar" && out.ServiceNodes[1].Node == "foo")
-		})
 
 		args.Filter = "ServiceMeta.version == 2"
-		out = new(structs.IndexedServiceNodes)
+		out = new(structs.IndexedServices)
-		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &args, &out))
-		require.Len(t, out.ServiceNodes, 1)
+		require.Len(t, out.Services, 4)
-		require.Equal(t, "foo", out.ServiceNodes[0].Node)
+		require.Len(t, out.Services["redis"], 1)
+		require.Len(t, out.Services["web"], 2)
+		require.Len(t, out.Services["critical"], 1)
+		require.Len(t, out.Services["warning"], 1)
 	})
 
 	t.Run("NodeServices", func(t *testing.T) {
@@ -1022,6 +1023,46 @@ func TestCatalog_RPC_Filter(t *testing.T) {
 		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &args, &out))
 		require.Len(t, out.NodeServices.Services, 1)
 	})
+
+	t.Run("NodeServiceList", func(t *testing.T) {
+		args := structs.NodeSpecificRequest{
+			Datacenter:   "dc1",
+			Node:         "baz",
+			QueryOptions: structs.QueryOptions{Filter: "Service == web"},
+		}
+
+		out := new(structs.IndexedNodeServiceList)
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &args, &out))
+		require.Len(t, out.NodeServices.Services, 2)
+
+		args.Filter = "Service == web and Meta.version == 2"
+		out = new(structs.IndexedNodeServiceList)
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &args, &out))
+		require.Len(t, out.NodeServices.Services, 1)
+	})
+
+	t.Run("ServiceNodes", func(t *testing.T) {
+		args := structs.ServiceSpecificRequest{
+			Datacenter:   "dc1",
+			ServiceName:  "redis",
+			QueryOptions: structs.QueryOptions{Filter: "ServiceMeta.version == 1"},
+		}
+
+		out := new(structs.IndexedServiceNodes)
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
+		require.Len(t, out.ServiceNodes, 2)
+		require.Condition(t, func() bool {
+			return (out.ServiceNodes[0].Node == "foo" && out.ServiceNodes[1].Node == "bar") ||
+				(out.ServiceNodes[0].Node == "bar" && out.ServiceNodes[1].Node == "foo")
+		})
+
+		args.Filter = "ServiceMeta.version == 2"
+		out = new(structs.IndexedServiceNodes)
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &args, &out))
+		require.Len(t, out.ServiceNodes, 1)
+		require.Equal(t, "foo", out.ServiceNodes[0].Node)
+	})
+
 }
 
 func TestCatalog_ListNodes_StaleRead(t *testing.T) {
@@ -1332,6 +1373,7 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
 		Datacenter: "dc1",
 	}
 
+	readToken := token("read")
 	t.Run("deny", func(t *testing.T) {
 		args.Token = token("deny")
 
@@ -1348,7 +1390,7 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
 	})
 
 	t.Run("allow", func(t *testing.T) {
-		args.Token = token("read")
+		args.Token = readToken
 
 		var reply structs.IndexedNodes
 		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply); err != nil {
@@ -1361,6 +1403,67 @@ func TestCatalog_ListNodes_ACLFilter(t *testing.T) {
 			t.Fatal("ResultsFilteredByACLs should not true")
 		}
 	})
+
+	// Register additional node
+	regArgs := &structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       "foo",
+		Address:    "127.0.0.1",
+		WriteRequest: structs.WriteRequest{
+			Token: "root",
+		},
+	}
+
+	var out struct{}
+	require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
+
+	bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", s1.config.NodeName)
+	const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		var reply structs.IndexedNodes
+		args = structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  readToken,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodes{}
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
+		require.Equal(t, 1, len(reply.Nodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		var reply structs.IndexedNodes
+		args = structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  readToken,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodes{}
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
+		require.Empty(t, reply.Nodes)
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		var reply structs.IndexedNodes
+		args = structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodes{}
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &reply))
+		require.Empty(t, reply.Nodes)
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func Benchmark_Catalog_ListNodes(t *testing.B) {
@@ -2758,6 +2861,14 @@ service "foo" {
 node_prefix "" {
 	policy = "read"
 }
+
+node "node-deny" {
+	policy = "deny"
+}
+
+service "service-deny" {
+	policy = "deny"
+}
 `
 	token = createToken(t, codec, rules)
 
@@ -2915,23 +3026,76 @@ func TestCatalog_ListServices_FilterACL(t *testing.T) {
 	defer codec.Close()
 	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken("root"))
 
-	opt := structs.DCSpecificRequest{
+	t.Run("request with user token without filter param sets ResultsFilteredByACLs equal to true", func(t *testing.T) {
-		Datacenter:   "dc1",
+		req := structs.DCSpecificRequest{
-		QueryOptions: structs.QueryOptions{Token: token},
+			Datacenter:   "dc1",
-	}
+			QueryOptions: structs.QueryOptions{Token: token},
-	reply := structs.IndexedServices{}
+		}
-	if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &opt, &reply); err != nil {
+		reply := structs.IndexedServices{}
-		t.Fatalf("err: %s", err)
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
-	}
+			t.Fatalf("err: %s", err)
-	if _, ok := reply.Services["foo"]; !ok {
+		}
-		t.Fatalf("bad: %#v", reply.Services)
+		if _, ok := reply.Services["foo"]; !ok {
-	}
+			t.Fatalf("bad: %#v", reply.Services)
-	if _, ok := reply.Services["bar"]; ok {
+		}
-		t.Fatalf("bad: %#v", reply.Services)
+		if _, ok := reply.Services["bar"]; ok {
-	}
+			t.Fatalf("bad: %#v", reply.Services)
-	if !reply.QueryMeta.ResultsFilteredByACLs {
+		}
-		t.Fatal("ResultsFilteredByACLs should be true")
+		if !reply.QueryMeta.ResultsFilteredByACLs {
-	}
+			t.Fatal("ResultsFilteredByACLs should be true")
+		}
+	})
+
+	const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
+	const bexpNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.Services))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
@@ -2982,11 +3146,80 @@ func TestCatalog_ServiceNodes_FilterACL(t *testing.T) {
 	}
 	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
-	// We've already proven that we call the ACL filtering function so we
+	bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", srv.config.NodeName)
-	// test node filtering down in acl.go for node cases. This also proves
+	const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
-	// that we respect the version 8 ACL flag, since the test server sets
+
-	// that to false (the regression value of *not* changing this is better
+	// Register a service of the same name on the denied node
-	// for now until we change the sense of the version 8 ACL flag).
+	regArg := structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       "node-deny",
+		Address:    "127.0.0.1",
+		Service: &structs.NodeService{
+			ID:      "foo",
+			Service: "foo",
+		},
+		Check: &structs.HealthCheck{
+			CheckID:   "service:foo",
+			Name:      "service:foo",
+			ServiceID: "foo",
+			Status:    api.HealthPassing,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt = structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedServiceNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.ServiceNodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt = structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedServiceNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.ServiceNodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		opt = structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedServiceNodes{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ServiceNodes", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.ServiceNodes))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestCatalog_NodeServices_ACL(t *testing.T) {
@@ -3075,6 +3308,139 @@ func TestCatalog_NodeServices_FilterACL(t *testing.T) {
 	svc, ok := reply.NodeServices.Services["foo"]
 	require.True(t, ok)
 	require.Equal(t, "foo", svc.ID)
+
+	const bexprMatchingUserTokenPermissions = "Service matches `f.*`"
+	const bexpNotMatchingUserTokenPermissions = "Service matches `b.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.NodeServices.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.NodeServices.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServices{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServices", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Nil(t, reply.NodeServices)
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
+}
+
+func TestCatalog_NodeServicesList_FilterACL(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+	dir, token, srv, codec := testACLFilterServer(t)
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer codec.Close()
+	testrpc.WaitForTestAgent(t, srv.RPC, "dc1", testrpc.WithToken("root"))
+
+	opt := structs.NodeSpecificRequest{
+		Datacenter:   "dc1",
+		Node:         srv.config.NodeName,
+		QueryOptions: structs.QueryOptions{Token: token},
+	}
+
+	var reply structs.IndexedNodeServiceList
+	require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &opt, &reply))
+
+	require.NotNil(t, reply.NodeServices)
+	require.Len(t, reply.NodeServices.Services, 1)
+
+	const bexprMatchingUserTokenPermissions = "Service matches `f.*`"
+	const bexpNotMatchingUserTokenPermissions = "Service matches `b.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServiceList{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.NodeServices.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServiceList{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.NodeServices.Services))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply = structs.IndexedNodeServiceList{}
+		if err := msgpackrpc.CallWithCodec(codec, "Catalog.NodeServiceList", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Empty(t, reply.NodeServices.Services)
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestCatalog_GatewayServices_TerminatingGateway(t *testing.T) {

agent/consul/config_endpoint.go

@@ -9,7 +9,6 @@ import (
 	"time"
 
 	metrics "github.com/armon/go-metrics"
-	"github.com/armon/go-metrics/prometheus"
 	hashstructure_v2 "github.com/mitchellh/hashstructure/v2"
 
 	"github.com/hashicorp/go-bexpr"
@@ -22,33 +21,6 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 )
 
-var ConfigSummaries = []prometheus.SummaryDefinition{
-	{
-		Name: []string{"config_entry", "apply"},
-		Help: "",
-	},
-	{
-		Name: []string{"config_entry", "get"},
-		Help: "",
-	},
-	{
-		Name: []string{"config_entry", "list"},
-		Help: "",
-	},
-	{
-		Name: []string{"config_entry", "listAll"},
-		Help: "",
-	},
-	{
-		Name: []string{"config_entry", "delete"},
-		Help: "",
-	},
-	{
-		Name: []string{"config_entry", "resolve_service_config"},
-		Help: "",
-	},
-}
-
 // The ConfigEntry endpoint is used to query centralized config information
 type ConfigEntry struct {
 	srv    *Server
@@ -280,7 +252,14 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe
 				return err
 			}
 
-			// Filter the entries returned by ACL permissions.
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
 			filteredEntries := make([]structs.ConfigEntry, 0, len(entries))
 			for _, entry := range entries {
 				if err := entry.CanRead(authz); err != nil {

agent/consul/config_endpoint_test.go

@@ -783,7 +783,7 @@ service "foo" {
 }
 operator = "read"
 `
-	id := createToken(t, codec, rules)
+	token := createToken(t, codec, rules)
 
 	// Create some dummy service/proxy configs to be looked up.
 	state := s1.fsm.State()
@@ -804,7 +804,7 @@ operator = "read"
 	args := structs.ConfigEntryQuery{
 		Kind:         structs.ServiceDefaults,
 		Datacenter:   s1.config.Datacenter,
-		QueryOptions: structs.QueryOptions{Token: id},
+		QueryOptions: structs.QueryOptions{Token: token},
 	}
 	var out structs.IndexedConfigEntries
 	err := msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out)
@@ -828,6 +828,58 @@ operator = "read"
 	require.Equal(t, structs.ProxyConfigGlobal, proxyConf.Name)
 	require.Equal(t, structs.ProxyDefaults, proxyConf.Kind)
 	require.False(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+
+	// ensure ACL filtering occurs before bexpr filtering.
+	const bexprMatchingUserTokenPermissions = "Name matches `f.*`"
+	const bexprNotMatchingUserTokenPermissions = "Name matches `db.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		args = structs.ConfigEntryQuery{
+			Kind:       structs.ServiceDefaults,
+			Datacenter: s1.config.Datacenter,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		var reply structs.IndexedConfigEntries
+		err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
+		require.NoError(t, err)
+		require.Equal(t, 1, len(reply.Entries))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		args = structs.ConfigEntryQuery{
+			Kind:       structs.ServiceDefaults,
+			Datacenter: s1.config.Datacenter,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		var reply structs.IndexedConfigEntries
+		err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.Entries))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		args = structs.ConfigEntryQuery{
+			Kind:       structs.ServiceDefaults,
+			Datacenter: s1.config.Datacenter,
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		var reply structs.IndexedConfigEntries
+		err = msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.Entries))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestConfigEntry_ListAll_ACLDeny(t *testing.T) {

agent/consul/health_endpoint.go

@@ -63,18 +63,23 @@ func (h *Health) ChecksInState(args *structs.ChecksInStateRequest,
 			}
 			reply.Index, reply.HealthChecks = index, checks
 
-			raw, err := filter.Execute(reply.HealthChecks)
+			// Note: we filter the results with ACLs *before* applying the user-supplied
-			if err != nil {
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
-			reply.HealthChecks = raw.(structs.HealthChecks)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
+			raw, err := filter.Execute(reply.HealthChecks)
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			if err != nil {
-			// results that would be filtered out even if the user did have permission.
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
+			reply.HealthChecks = raw.(structs.HealthChecks)
 
 			return h.srv.sortNodesByDistanceFrom(args.Source, reply.HealthChecks)
 		})
@@ -111,18 +116,23 @@ func (h *Health) NodeChecks(args *structs.NodeSpecificRequest,
 			}
 			reply.Index, reply.HealthChecks = index, checks
 
-			raw, err := filter.Execute(reply.HealthChecks)
+			// Note: we filter the results with ACLs *before* applying the user-supplied
-			if err != nil {
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := h.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
-			reply.HealthChecks = raw.(structs.HealthChecks)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
+			raw, err := filter.Execute(reply.HealthChecks)
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			if err != nil {
-			// results that would be filtered out even if the user did have permission.
-			if err := h.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
+			reply.HealthChecks = raw.(structs.HealthChecks)
 
 			return nil
 		})
@@ -303,6 +313,18 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 					thisReply.Nodes = nodeMetaFilter(arg.NodeMetaFilters, thisReply.Nodes)
 				}
 
+				// Note: we filter the results with ACLs *before* applying the user-supplied
+				// bexpr filter to ensure that the user can only run expressions on data that
+				// they have access to.  This is a security measure to prevent users from
+				// running arbitrary expressions on data they don't have access to.
+				// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+				// that results they don't have access to have been removed.  If they were
+				// also allowed to run the bexpr filter on the data, they could potentially
+				// infer the specific attributes of data they don't have access to.
+				if err := h.srv.filterACL(arg.Token, &thisReply); err != nil {
+					return err
+				}
+
 				raw, err := filter.Execute(thisReply.Nodes)
 				if err != nil {
 					return err
@@ -310,13 +332,6 @@ func (h *Health) ServiceNodes(args *structs.ServiceSpecificRequest, reply *struc
 				filteredNodes := raw.(structs.CheckServiceNodes)
 				thisReply.Nodes = filteredNodes.Filter(structs.CheckServiceNodeFilterOptions{FilterType: arg.HealthFilterType})
 
-				// Note: we filter the results with ACLs *after* applying the user-supplied
-				// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
-				// results that would be filtered out even if the user did have permission.
-				if err := h.srv.filterACL(arg.Token, &thisReply); err != nil {
-					return err
-				}
-
 				if err := h.srv.sortNodesByDistanceFrom(arg.Source, thisReply.Nodes); err != nil {
 					return err
 				}

agent/consul/health_endpoint_test.go

@@ -1527,11 +1527,62 @@ func TestHealth_NodeChecks_FilterACL(t *testing.T) {
 	require.True(t, found, "bad: %#v", reply.HealthChecks)
 	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
-	// We've already proven that we call the ACL filtering function so we
+	const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
-	// test node filtering down in acl.go for node cases. This also proves
+	const bexprNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
-	// that we respect the version 8 ACL flag, since the test server sets
+
-	// that to false (the regression value of *not* changing this is better
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
-	// for now until we change the sense of the version 8 ACL flag).
+		opt := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+
+		if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+
+		if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		opt := structs.NodeSpecificRequest{
+			Datacenter: "dc1",
+			Node:       srv.config.NodeName,
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+
+		if err := msgpackrpc.CallWithCodec(codec, "Health.NodeChecks", &opt, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.HealthChecks))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
@@ -1571,11 +1622,77 @@ func TestHealth_ServiceChecks_FilterACL(t *testing.T) {
 	require.Empty(t, reply.HealthChecks)
 	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
-	// We've already proven that we call the ACL filtering function so we
+	// Register a service of the same name on the denied node
-	// test node filtering down in acl.go for node cases. This also proves
+	regArg := structs.RegisterRequest{
-	// that we respect the version 8 ACL flag, since the test server sets
+		Datacenter: "dc1",
-	// that to false (the regression value of *not* changing this is better
+		Node:       "node-deny",
-	// for now until we change the sense of the version 8 ACL flag).
+		Address:    "127.0.0.1",
+		Service: &structs.NodeService{
+			ID:      "foo",
+			Service: "foo",
+		},
+		Check: &structs.HealthCheck{
+			CheckID:   "service:foo",
+			Name:      "service:foo",
+			ServiceID: "foo",
+			Status:    api.HealthPassing,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
+	const bexprNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
+		require.NoError(t, err)
+
+		require.Equal(t, 1, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceChecks", &opt, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.HealthChecks))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
@@ -1607,11 +1724,77 @@ func TestHealth_ServiceNodes_FilterACL(t *testing.T) {
 	require.Empty(t, reply.Nodes)
 	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
-	// We've already proven that we call the ACL filtering function so we
+	// Register a service of the same name on the denied node
-	// test node filtering down in acl.go for node cases. This also proves
+	regArg := structs.RegisterRequest{
-	// that we respect the version 8 ACL flag, since the test server sets
+		Datacenter: "dc1",
-	// that to false (the regression value of *not* changing this is better
+		Node:       "node-deny",
-	// for now until we change the sense of the version 8 ACL flag).
+		Address:    "127.0.0.1",
+		Service: &structs.NodeService{
+			ID:      "foo",
+			Service: "foo",
+		},
+		Check: &structs.HealthCheck{
+			CheckID:   "service:foo",
+			Name:      "service:foo",
+			ServiceID: "foo",
+			Status:    api.HealthPassing,
+		},
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+	if err := msgpackrpc.CallWithCodec(codec, "Catalog.Register", &regArg, nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	const bexprMatchingUserTokenPermissions = "Service.Service matches `f.*`"
+	const bexprNotMatchingUserTokenPermissions = "Node.Node matches `node-deny.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedCheckServiceNodes{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
+		require.NoError(t, err)
+
+		require.Equal(t, 1, len(reply.Nodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedCheckServiceNodes{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.Nodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		opt := structs.ServiceSpecificRequest{
+			Datacenter:  "dc1",
+			ServiceName: "foo",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedCheckServiceNodes{}
+		err := msgpackrpc.CallWithCodec(codec, "Health.ServiceNodes", &opt, &reply)
+		require.NoError(t, err)
+		require.Zero(t, len(reply.Nodes))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestHealth_ChecksInState_FilterACL(t *testing.T) {
@@ -1647,11 +1830,59 @@ func TestHealth_ChecksInState_FilterACL(t *testing.T) {
 	require.True(t, found, "missing service 'foo': %#v", reply.HealthChecks)
 	require.True(t, reply.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 
-	// We've already proven that we call the ACL filtering function so we
+	const bexprMatchingUserTokenPermissions = "ServiceName matches `f.*`"
-	// test node filtering down in acl.go for node cases. This also proves
+	const bexprNotMatchingUserTokenPermissions = "ServiceName matches `b.*`"
-	// that we respect the version 8 ACL flag, since the test server sets
+
-	// that to false (the regression value of *not* changing this is better
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
-	// for now until we change the sense of the version 8 ACL flag).
+		req := structs.ChecksInStateRequest{
+			Datacenter: "dc1",
+			State:      api.HealthPassing,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.ChecksInStateRequest{
+			Datacenter: "dc1",
+			State:      api.HealthPassing,
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.HealthChecks))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would normally match but without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := structs.ChecksInStateRequest{
+			Datacenter: "dc1",
+			State:      api.HealthPassing,
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexprNotMatchingUserTokenPermissions,
+			},
+		}
+		reply := structs.IndexedHealthChecks{}
+		if err := msgpackrpc.CallWithCodec(codec, "Health.ChecksInState", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.HealthChecks))
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestHealth_RPC_Filter(t *testing.T) {

agent/consul/intention_endpoint.go

@@ -550,18 +550,24 @@ func (s *Intention) List(args *structs.IntentionListRequest, reply *structs.Inde
 			} else {
 				reply.DataOrigin = structs.IntentionDataOriginLegacy
 			}
-			raw, err := filter.Execute(reply.Intentions)
+
-			if err != nil {
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := s.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
-			reply.Intentions = raw.(structs.Intentions)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
+			raw, err := filter.Execute(reply.Intentions)
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
+			if err != nil {
-			// results that would be filtered out even if the user did have permission.
-			if err := s.srv.filterACL(args.Token, reply); err != nil {
 				return err
 			}
+			reply.Intentions = raw.(structs.Intentions)
 
 			return nil
 		},

agent/consul/intention_endpoint_test.go

@@ -1639,6 +1639,11 @@ func TestIntentionList_acl(t *testing.T) {
 	token, err := upsertTestTokenWithPolicyRules(codec, TestDefaultInitialManagementToken, "dc1", `service_prefix "foo" { policy = "write" }`)
 	require.NoError(t, err)
 
+	const (
+		bexprMatch   = "DestinationName matches `f.*`"
+		bexprNoMatch = "DestinationName matches `nomatch.*`"
+	)
+
 	// Create a few records
 	for _, name := range []string{"foobar", "bar", "baz"} {
 		ixn := structs.IntentionRequest{
@@ -1691,12 +1696,29 @@ func TestIntentionList_acl(t *testing.T) {
 		require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 	})
 
-	t.Run("filtered", func(t *testing.T) {
+	// maskResultsFilteredByACLs() in rpc.go sets ResultsFilteredByACLs to false if the token is an empty string
+	// after resp.QueryMeta.ResultsFilteredByACLs has been determined to be true from filterACLs().
+	t.Run("filtered with no token should return no results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := &structs.IntentionListRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Filter: bexprMatch,
+			},
+		}
+
+		var resp structs.IndexedIntentions
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
+		require.Len(t, resp.Intentions, 0)
+		require.False(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
+	})
+
+	// has access to everything
+	t.Run("filtered with initial management token should return 1 and ResultsFilteredByACLs equal to false", func(t *testing.T) {
 		req := &structs.IntentionListRequest{
 			Datacenter: "dc1",
 			QueryOptions: structs.QueryOptions{
 				Token:  TestDefaultInitialManagementToken,
-				Filter: "DestinationName == foobar",
+				Filter: bexprMatch,
 			},
 		}
 
@@ -1705,6 +1727,54 @@ func TestIntentionList_acl(t *testing.T) {
 		require.Len(t, resp.Intentions, 1)
 		require.False(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be false")
 	})
+
+	// ResultsFilteredByACLs should reflect user does not have access to read all intentions but has access to some.
+	t.Run("filtered with user token whose permissions match filter should return 1 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := &structs.IntentionListRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token.SecretID,
+				Filter: bexprMatch,
+			},
+		}
+
+		var resp structs.IndexedIntentions
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
+		require.Len(t, resp.Intentions, 1)
+		require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	// ResultsFilteredByACLs need to act as though no filter was applied.
+	t.Run("filtered with user token whose permissions do match filter should return 0 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := &structs.IntentionListRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token.SecretID,
+				Filter: bexprNoMatch,
+			},
+		}
+
+		var resp structs.IndexedIntentions
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
+		require.Len(t, resp.Intentions, 0)
+		require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
+
+	// ResultsFilteredByACLs should reflect user does not have access to read any intentions
+	t.Run("filtered with anonymous token should return 0 and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := &structs.IntentionListRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  "anonymous",
+				Filter: bexprMatch,
+			},
+		}
+
+		var resp structs.IndexedIntentions
+		require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.List", req, &resp))
+		require.Len(t, resp.Intentions, 0)
+		require.True(t, resp.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
+	})
 }
 
 // Test basic matching. We don't need to exhaustively test inputs since this

agent/consul/internal_endpoint.go

@@ -117,6 +117,18 @@ func (m *Internal) NodeDump(args *structs.DCSpecificRequest,
 			}
 			reply.Index = maxIndex
 
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := m.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
 			raw, err := filter.Execute(reply.Dump)
 			if err != nil {
 				return fmt.Errorf("could not filter local node dump: %w", err)
@@ -129,13 +141,6 @@ func (m *Internal) NodeDump(args *structs.DCSpecificRequest,
 			}
 			reply.ImportedDump = importedRaw.(structs.NodeDump)
 
-			// Note: we filter the results with ACLs *after* applying the user-supplied
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
-			// results that would be filtered out even if the user did have permission.
-			if err := m.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
-
 			return nil
 		})
 }
@@ -235,13 +240,26 @@ func (m *Internal) ServiceDump(args *structs.ServiceDumpRequest, reply *structs.
 					}
 				}
 				reply.Index = maxIndex
-				raw, err := filter.Execute(reply.Nodes)
-				if err != nil {
-					return fmt.Errorf("could not filter local service dump: %w", err)
-				}
-				reply.Nodes = raw.(structs.CheckServiceNodes)
 			}
 
+			// Note: we filter the results with ACLs *before* applying the user-supplied
+			// bexpr filter to ensure that the user can only run expressions on data that
+			// they have access to.  This is a security measure to prevent users from
+			// running arbitrary expressions on data they don't have access to.
+			// QueryMeta.ResultsFilteredByACLs being true already indicates to the user
+			// that results they don't have access to have been removed.  If they were
+			// also allowed to run the bexpr filter on the data, they could potentially
+			// infer the specific attributes of data they don't have access to.
+			if err := m.srv.filterACL(args.Token, reply); err != nil {
+				return err
+			}
+
+			raw, err := filter.Execute(reply.Nodes)
+			if err != nil {
+				return fmt.Errorf("could not filter local service dump: %w", err)
+			}
+			reply.Nodes = raw.(structs.CheckServiceNodes)
+
 			if !args.NodesOnly {
 				importedRaw, err := filter.Execute(reply.ImportedNodes)
 				if err != nil {
@@ -249,12 +267,6 @@ func (m *Internal) ServiceDump(args *structs.ServiceDumpRequest, reply *structs.
 				}
 				reply.ImportedNodes = importedRaw.(structs.CheckServiceNodes)
 			}
-			// Note: we filter the results with ACLs *after* applying the user-supplied
-			// bexpr filter, to ensure QueryMeta.ResultsFilteredByACLs does not include
-			// results that would be filtered out even if the user did have permission.
-			if err := m.srv.filterACL(args.Token, reply); err != nil {
-				return err
-			}
 
 			return nil
 		})

agent/consul/internal_endpoint_test.go

@@ -656,11 +656,73 @@ func TestInternal_NodeDump_FilterACL(t *testing.T) {
 		t.Fatal("ResultsFilteredByACLs should be true")
 	}
 
-	// We've already proven that we call the ACL filtering function so we
+	// need to ensure that ACLs are filtered prior to bexprFiltering
-	// test node filtering down in acl.go for node cases. This also proves
+	// Register additional node
-	// that we respect the version 8 ACL flag, since the test server sets
+	regArgs := &structs.RegisterRequest{
-	// that to false (the regression value of *not* changing this is better
+		Datacenter: "dc1",
-	// for now until we change the sense of the version 8 ACL flag).
+		Node:       "foo",
+		Address:    "127.0.0.1",
+		WriteRequest: structs.WriteRequest{
+			Token: "root",
+		},
+	}
+
+	var out struct{}
+	require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
+
+	bexprMatchingUserTokenPermissions := fmt.Sprintf("Node matches `%s.*`", srv.config.NodeName)
+	const bexpNotMatchingUserTokenPermissions = "Node matches `node-deny.*`"
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodeDump{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.Dump))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodeDump{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.Dump))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would match only record without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  "",
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodeDump{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.NodeDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Empty(t, reply.Dump)
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestInternal_EventFire_Token(t *testing.T) {
@@ -1064,6 +1126,113 @@ func TestInternal_ServiceDump_ACL(t *testing.T) {
 		require.Empty(t, out.Gateways)
 		require.True(t, out.QueryMeta.ResultsFilteredByACLs, "ResultsFilteredByACLs should be true")
 	})
+
+	// need to ensure that ACLs are filtered prior to bexprFiltering
+	// Register additional node
+	regArgs := &structs.RegisterRequest{
+		Datacenter: "dc1",
+		Node:       "node-deny",
+		ID:         types.NodeID("e0155642-135d-4739-9853-b1ee6c9f945b"),
+		Address:    "192.18.1.2",
+		Service: &structs.NodeService{
+			Kind:    structs.ServiceKindTypical,
+			ID:      "memcached",
+			Service: "memcached",
+			Port:    5678,
+		},
+		Check: &structs.HealthCheck{
+			Name:      "memcached check",
+			Status:    api.HealthPassing,
+			ServiceID: "memcached",
+		},
+		WriteRequest: structs.WriteRequest{
+			Token: "root",
+		},
+	}
+
+	var out struct{}
+	require.NoError(t, msgpackrpc.CallWithCodec(codec, "Catalog.Register", regArgs, &out))
+
+	const (
+		bexprMatchingUserTokenPermissions   = "Service.Service matches `redis.*`"
+		bexpNotMatchingUserTokenPermissions = "Node.Node matches `node-deny.*`"
+	)
+
+	t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		token := tokenWithRules(t, `
+			node "node-deny" {
+				policy = "deny"
+			}
+			node "node1" {
+				policy = "read"
+			}
+			service "redis" {
+				policy = "read"
+			}
+		`)
+		var reply structs.IndexedNodesWithGateways
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexprMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodesWithGateways{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Equal(t, 1, len(reply.Nodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+		token := tokenWithRules(t, `
+			node "node-deny" {
+				policy = "deny"
+			}
+			node "node1" {
+				policy = "read"
+			}
+			service "redis" {
+				policy = "read"
+			}
+		`)
+		var reply structs.IndexedNodesWithGateways
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  token,
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodesWithGateways{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Zero(t, len(reply.Nodes))
+		require.True(t, reply.ResultsFilteredByACLs)
+	})
+
+	t.Run("request with filter that would match only record without any token returns zero results and ResultsFilteredByACLs equal to false", func(t *testing.T) {
+		var reply structs.IndexedNodesWithGateways
+		req := structs.DCSpecificRequest{
+			Datacenter: "dc1",
+			QueryOptions: structs.QueryOptions{
+				Token:  "", // no token
+				Filter: bexpNotMatchingUserTokenPermissions,
+			},
+		}
+
+		reply = structs.IndexedNodesWithGateways{}
+		if err := msgpackrpc.CallWithCodec(codec, "Internal.ServiceDump", &req, &reply); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		require.Empty(t, reply.Nodes)
+		require.False(t, reply.ResultsFilteredByACLs)
+	})
 }
 
 func TestInternal_GatewayServiceDump_Terminating(t *testing.T) {

agent/dns_test.go

@@ -2367,7 +2367,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
 		},
 	}
 
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"  `)
 	if trimmed := trimUDPResponse(req, resp, cfg.DNSUDPAnswerLimit); trimmed {
 		t.Fatalf("Bad %#v", *resp)
 	}
@@ -2400,7 +2400,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < cfg.DNSUDPAnswerLimit+1; i++ {
@@ -2439,7 +2439,7 @@ func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimLimitWithNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < cfg.DNSUDPAnswerLimit+1; i++ {
@@ -2486,7 +2486,7 @@ func TestDNS_trimUDPResponse_TrimLimitWithNS(t *testing.T) {
 }
 
 func TestDNS_trimTCPResponse_TrimLimitWithNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < 5000; i++ {
@@ -2542,7 +2542,7 @@ func loadRuntimeConfig(t *testing.T, hcl string) *config.RuntimeConfig {
 }
 
 func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"  `)
 
 	req, resp := &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < 100; i++ {
@@ -2594,7 +2594,7 @@ func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimSizeEDNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp := &dns.Msg{}, &dns.Msg{}
 
@@ -2672,7 +2672,7 @@ func TestDNS_trimUDPResponse_TrimSizeEDNS(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimSizeMaxSize(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"             `)
 
 	resp := &dns.Msg{}
 

agent/proxycfg-glue/internal_service_dump.go

@@ -81,19 +81,21 @@ func (s *serverInternalServiceDump) Notify(ctx context.Context, req *structs.Ser
 				return 0, nil, err
 			}
 
+			totalNodeLength := len(nodes)
+			aclfilter.New(authz, s.deps.Logger).Filter(&nodes)
+
 			raw, err := filter.Execute(nodes)
 			if err != nil {
 				return 0, nil, fmt.Errorf("could not filter local service dump: %w", err)
 			}
 			nodes = raw.(structs.CheckServiceNodes)
 
-			aclfilter.New(authz, s.deps.Logger).Filter(&nodes)
-
 			return idx, &structs.IndexedCheckServiceNodes{
 				Nodes: nodes,
 				QueryMeta: structs.QueryMeta{
-					Index:   idx,
+					Index:                 idx,
-					Backend: structs.QueryBackendBlocking,
+					Backend:               structs.QueryBackendBlocking,
+					ResultsFilteredByACLs: totalNodeLength != len(nodes),
 				},
 			}, nil
 		},

agent/proxycfg-glue/internal_service_dump_test.go

@@ -55,6 +55,10 @@ func TestServerInternalServiceDump(t *testing.T) {
 				Service: "web",
 				Kind:    structs.ServiceKindTypical,
 			},
+			{
+				Service: "web-deny",
+				Kind:    structs.ServiceKindTypical,
+			},
 			{
 				Service: "db",
 				Kind:    structs.ServiceKindTypical,
@@ -67,14 +71,14 @@ func TestServerInternalServiceDump(t *testing.T) {
 			}))
 		}
 
-		authz := newStaticResolver(
+		policyAuth := policyAuthorizer(t, `
-			policyAuthorizer(t, `
 				service "mgw" { policy = "read" }
 				service "web" { policy = "read" }
+				service "web-deny" { policy = "deny" }
 				service "db"  { policy = "read" }
 				node_prefix "node-" { policy = "read" }
-			`),
+			`)
-		)
+		authz := newStaticResolver(policyAuth)
 
 		dataSource := ServerInternalServiceDump(ServerDataSourceDeps{
 			GetStore:    func() Store { return store },
@@ -121,6 +125,42 @@ func TestServerInternalServiceDump(t *testing.T) {
 			result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
 			require.Empty(t, result.Nodes)
 		})
+
+		const (
+			bexprMatchingUserTokenPermissions   = "Service.Service matches `web.*`"
+			bexpNotMatchingUserTokenPermissions = "Service.Service matches `mgw.*`"
+		)
+
+		authz.SwapAuthorizer(policyAuthorizer(t, `
+				service "mgw" { policy = "deny" }
+				service "web" { policy = "read" }
+				service "web-deny" { policy = "deny" }
+				service "db"  { policy = "read" }
+				node_prefix "node-" { policy = "read" }
+			`))
+
+		t.Run("request with filter that matches token permissions returns 1 result and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+			eventCh := make(chan proxycfg.UpdateEvent)
+			require.NoError(t, dataSource.Notify(ctx, &structs.ServiceDumpRequest{
+				QueryOptions: structs.QueryOptions{Filter: bexprMatchingUserTokenPermissions},
+			}, "", eventCh))
+
+			result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
+			require.Len(t, result.Nodes, 1)
+			require.Equal(t, "web", result.Nodes[0].Service.Service)
+			require.True(t, result.ResultsFilteredByACLs)
+		})
+
+		t.Run("request with filter that does not match token permissions returns 0 results and ResultsFilteredByACLs equal to true", func(t *testing.T) {
+			eventCh := make(chan proxycfg.UpdateEvent)
+			require.NoError(t, dataSource.Notify(ctx, &structs.ServiceDumpRequest{
+				QueryOptions: structs.QueryOptions{Filter: bexpNotMatchingUserTokenPermissions},
+			}, "", eventCh))
+
+			result := getEventResult[*structs.IndexedCheckServiceNodes](t, eventCh)
+			require.Len(t, result.Nodes, 0)
+			require.True(t, result.ResultsFilteredByACLs)
+		})
 	})
 }
 

agent/proxycfg/api_gateway.go

@@ -476,16 +476,12 @@ func (h *handlerAPIGateway) handleRouteConfigUpdate(ctx context.Context, u Updat
 			cancelUpstream()
 			delete(snap.APIGateway.WatchedUpstreams[upstreamID], targetID)
 			delete(snap.APIGateway.WatchedUpstreamEndpoints[upstreamID], targetID)
-
-			if targetUID := NewUpstreamIDFromTargetID(targetID); targetUID.Peer != "" {
-				snap.APIGateway.PeerUpstreamEndpoints.CancelWatch(targetUID)
-				snap.APIGateway.UpstreamPeerTrustBundles.CancelWatch(targetUID.Peer)
-			}
 		}
 
 		cancelDiscoChain()
 		delete(snap.APIGateway.WatchedDiscoveryChains, upstreamID)
 	}
+	reconcilePeeringWatches(snap.APIGateway.DiscoveryChain, snap.APIGateway.UpstreamConfig, snap.APIGateway.PeeredUpstreams, snap.APIGateway.PeerUpstreamEndpoints, snap.APIGateway.UpstreamPeerTrustBundles)
 
 	return nil
 }

agent/proxycfg/connect_proxy.go

@@ -380,49 +380,7 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
 		//
 		// Clean up data
 		//
-
+		reconcilePeeringWatches(snap.ConnectProxy.DiscoveryChain, snap.ConnectProxy.UpstreamConfig, snap.ConnectProxy.PeeredUpstreams, snap.ConnectProxy.PeerUpstreamEndpoints, snap.ConnectProxy.UpstreamPeerTrustBundles)
-		peeredChainTargets := make(map[UpstreamID]struct{})
-		for _, discoChain := range snap.ConnectProxy.DiscoveryChain {
-			for _, target := range discoChain.Targets {
-				if target.Peer == "" {
-					continue
-				}
-				uid := NewUpstreamIDFromTargetID(target.ID)
-				peeredChainTargets[uid] = struct{}{}
-			}
-		}
-
-		validPeerNames := make(map[string]struct{})
-
-		// Iterate through all known endpoints and remove references to upstream IDs that weren't in the update
-		snap.ConnectProxy.PeerUpstreamEndpoints.ForEachKey(func(uid UpstreamID) bool {
-			// Peered upstream is explicitly defined in upstream config
-			if _, ok := snap.ConnectProxy.UpstreamConfig[uid]; ok {
-				validPeerNames[uid.Peer] = struct{}{}
-				return true
-			}
-			// Peered upstream came from dynamic source of imported services
-			if _, ok := seenUpstreams[uid]; ok {
-				validPeerNames[uid.Peer] = struct{}{}
-				return true
-			}
-			// Peered upstream came from a discovery chain target
-			if _, ok := peeredChainTargets[uid]; ok {
-				validPeerNames[uid.Peer] = struct{}{}
-				return true
-			}
-			snap.ConnectProxy.PeerUpstreamEndpoints.CancelWatch(uid)
-			return true
-		})
-
-		// Iterate through all known trust bundles and remove references to any unseen peer names
-		snap.ConnectProxy.UpstreamPeerTrustBundles.ForEachKey(func(peerName PeerName) bool {
-			if _, ok := validPeerNames[peerName]; !ok {
-				snap.ConnectProxy.UpstreamPeerTrustBundles.CancelWatch(peerName)
-			}
-			return true
-		})
-
 	case u.CorrelationID == intentionUpstreamsID:
 		resp, ok := u.Result.(*structs.IndexedServiceList)
 		if !ok {
@@ -490,18 +448,13 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
 				continue
 			}
 			if _, ok := seenUpstreams[uid]; !ok {
-				for targetID, cancelFn := range targets {
+				for _, cancelFn := range targets {
 					cancelFn()
-
-					targetUID := NewUpstreamIDFromTargetID(targetID)
-					if targetUID.Peer != "" {
-						snap.ConnectProxy.PeerUpstreamEndpoints.CancelWatch(targetUID)
-						snap.ConnectProxy.UpstreamPeerTrustBundles.CancelWatch(targetUID.Peer)
-					}
 				}
 				delete(snap.ConnectProxy.WatchedUpstreams, uid)
 			}
 		}
+		reconcilePeeringWatches(snap.ConnectProxy.DiscoveryChain, snap.ConnectProxy.UpstreamConfig, snap.ConnectProxy.PeeredUpstreams, snap.ConnectProxy.PeerUpstreamEndpoints, snap.ConnectProxy.UpstreamPeerTrustBundles)
 		for uid := range snap.ConnectProxy.WatchedUpstreamEndpoints {
 			if upstream, ok := snap.ConnectProxy.UpstreamConfig[uid]; ok && !upstream.CentrallyConfigured {
 				continue

agent/proxycfg/ingress_gateway.go

@@ -171,18 +171,13 @@ func (s *handlerIngressGateway) handleUpdate(ctx context.Context, u UpdateEvent,
 					delete(snap.IngressGateway.WatchedUpstreams[uid], targetID)
 					delete(snap.IngressGateway.WatchedUpstreamEndpoints[uid], targetID)
 					cancelUpstreamFn()
-
-					targetUID := NewUpstreamIDFromTargetID(targetID)
-					if targetUID.Peer != "" {
-						snap.IngressGateway.PeerUpstreamEndpoints.CancelWatch(targetUID)
-						snap.IngressGateway.UpstreamPeerTrustBundles.CancelWatch(targetUID.Peer)
-					}
 				}
 
 				cancelFn()
 				delete(snap.IngressGateway.WatchedDiscoveryChains, uid)
 			}
 		}
+		reconcilePeeringWatches(snap.IngressGateway.DiscoveryChain, snap.IngressGateway.UpstreamConfig, snap.IngressGateway.PeeredUpstreams, snap.IngressGateway.PeerUpstreamEndpoints, snap.IngressGateway.UpstreamPeerTrustBundles)
 
 		if err := s.watchIngressLeafCert(ctx, snap); err != nil {
 			return err

agent/proxycfg/state.go

@@ -13,12 +13,15 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/hashicorp/go-hclog"
 	"golang.org/x/time/rate"
 
+	"github.com/hashicorp/go-hclog"
+
 	cachetype "github.com/hashicorp/consul/agent/cache-types"
+	"github.com/hashicorp/consul/agent/proxycfg/internal/watch"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/logging"
+	"github.com/hashicorp/consul/proto/private/pbpeering"
 )
 
 const (
@@ -551,3 +554,48 @@ func watchMeshGateway(ctx context.Context, opts gatewayWatchOpts) error {
 		EnterpriseMeta: *structs.DefaultEnterpriseMetaInPartition(opts.key.Partition),
 	}, correlationId, opts.notifyCh)
 }
+
+func reconcilePeeringWatches(compiledDiscoveryChains map[UpstreamID]*structs.CompiledDiscoveryChain, upstreams map[UpstreamID]*structs.Upstream, peeredUpstreams map[UpstreamID]struct{}, peerUpstreamEndpoints watch.Map[UpstreamID, structs.CheckServiceNodes], upstreamPeerTrustBundles watch.Map[PeerName, *pbpeering.PeeringTrustBundle]) {
+
+	peeredChainTargets := make(map[UpstreamID]struct{})
+	for _, discoChain := range compiledDiscoveryChains {
+		for _, target := range discoChain.Targets {
+			if target.Peer == "" {
+				continue
+			}
+			uid := NewUpstreamIDFromTargetID(target.ID)
+			peeredChainTargets[uid] = struct{}{}
+		}
+	}
+
+	validPeerNames := make(map[string]struct{})
+
+	// Iterate through all known endpoints and remove references to upstream IDs that weren't in the update
+	peerUpstreamEndpoints.ForEachKey(func(uid UpstreamID) bool {
+		// Peered upstream is explicitly defined in upstream config
+		if _, ok := upstreams[uid]; ok {
+			validPeerNames[uid.Peer] = struct{}{}
+			return true
+		}
+		// Peered upstream came from dynamic source of imported services
+		if _, ok := peeredUpstreams[uid]; ok {
+			validPeerNames[uid.Peer] = struct{}{}
+			return true
+		}
+		// Peered upstream came from a discovery chain target
+		if _, ok := peeredChainTargets[uid]; ok {
+			validPeerNames[uid.Peer] = struct{}{}
+			return true
+		}
+		peerUpstreamEndpoints.CancelWatch(uid)
+		return true
+	})
+
+	// Iterate through all known trust bundles and remove references to any unseen peer names
+	upstreamPeerTrustBundles.ForEachKey(func(peerName PeerName) bool {
+		if _, ok := validPeerNames[peerName]; !ok {
+			upstreamPeerTrustBundles.CancelWatch(peerName)
+		}
+		return true
+	})
+}

agent/proxycfg/upstreams.go

@@ -102,6 +102,7 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u UpdateEv
 		if err := s.resetWatchesFromChain(ctx, uid, resp.Chain, upstreamsSnapshot); err != nil {
 			return err
 		}
+		reconcilePeeringWatches(upstreamsSnapshot.DiscoveryChain, upstreamsSnapshot.UpstreamConfig, upstreamsSnapshot.PeeredUpstreams, upstreamsSnapshot.PeerUpstreamEndpoints, upstreamsSnapshot.UpstreamPeerTrustBundles)
 
 	case strings.HasPrefix(u.CorrelationID, upstreamPeerWatchIDPrefix):
 		resp, ok := u.Result.(*structs.IndexedCheckServiceNodes)
@@ -301,12 +302,6 @@ func (s *handlerUpstreams) resetWatchesFromChain(
 		delete(snap.WatchedUpstreams[uid], targetID)
 		delete(snap.WatchedUpstreamEndpoints[uid], targetID)
 		cancelFn()
-
-		targetUID := NewUpstreamIDFromTargetID(targetID)
-		if targetUID.Peer != "" {
-			snap.PeerUpstreamEndpoints.CancelWatch(targetUID)
-			snap.UpstreamPeerTrustBundles.CancelWatch(targetUID.Peer)
-		}
 	}
 
 	var (
@@ -479,8 +474,8 @@ func (s *handlerUpstreams) watchUpstreamTarget(ctx context.Context, snap *Config
 	var entMeta acl.EnterpriseMeta
 	entMeta.Merge(opts.entMeta)
 
-	ctx, cancel := context.WithCancel(ctx)
+	peerCtx, cancel := context.WithCancel(ctx)
-	err := s.dataSources.Health.Notify(ctx, &structs.ServiceSpecificRequest{
+	err := s.dataSources.Health.Notify(peerCtx, &structs.ServiceSpecificRequest{
 		PeerName:   opts.peer,
 		Datacenter: opts.datacenter,
 		QueryOptions: structs.QueryOptions{
@@ -506,25 +501,25 @@ func (s *handlerUpstreams) watchUpstreamTarget(ctx context.Context, snap *Config
 		return nil
 	}
 
-	if ok := snap.PeerUpstreamEndpoints.IsWatched(uid); !ok {
+	if !snap.PeerUpstreamEndpoints.IsWatched(uid) {
 		snap.PeerUpstreamEndpoints.InitWatch(uid, cancel)
 	}
-
 	// Check whether a watch for this peer exists to avoid duplicates.
-	if ok := snap.UpstreamPeerTrustBundles.IsWatched(uid.Peer); !ok {
+
-		peerCtx, cancel := context.WithCancel(ctx)
+	if !snap.UpstreamPeerTrustBundles.IsWatched(uid.Peer) {
-		if err := s.dataSources.TrustBundle.Notify(peerCtx, &cachetype.TrustBundleReadRequest{
+		peerCtx2, cancel2 := context.WithCancel(ctx)
+		if err := s.dataSources.TrustBundle.Notify(peerCtx2, &cachetype.TrustBundleReadRequest{
 			Request: &pbpeering.TrustBundleReadRequest{
 				Name:      uid.Peer,
 				Partition: uid.PartitionOrDefault(),
 			},
 			QueryOptions: structs.QueryOptions{Token: s.token},
 		}, peerTrustBundleIDPrefix+uid.Peer, s.ch); err != nil {
-			cancel()
+			cancel2()
 			return fmt.Errorf("error while watching trust bundle for peer %q: %w", uid.Peer, err)
 		}
 
-		snap.UpstreamPeerTrustBundles.InitWatch(uid.Peer, cancel)
+		snap.UpstreamPeerTrustBundles.InitWatch(uid.Peer, cancel2)
 	}
 
 	return nil

agent/routine-leak-checker/leak_test.go

@@ -64,9 +64,7 @@ func setupPrimaryServer(t *testing.T) *agent.TestAgent {
 
 	config := `
 	   server = true
-		datacenter = "primary"
+		datacenter = "primary"	
-		primary_datacenter = "primary"
-		
 		connect {
 			enabled = true
 		}

agent/structs/acl.go

@@ -800,6 +800,11 @@ func (policies ACLPolicies) resolveWithCache(cache *ACLCaches, entConf *acl.Conf
 			continue
 		}
 
+		//pulling from the cache, we don't want to break any rules that are already in the cache
+		if entConf == nil {
+			entConf = &acl.Config{}
+		}
+		entConf.WarnOnDuplicateKey = true
 		p, err := acl.NewPolicyFromSource(policy.Rules, entConf, policy.EnterprisePolicyMeta())
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse %q: %v", policy.Name, err)

agent/structs/acl_test.go

@@ -403,7 +403,7 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "5d5653a1-2c2b-4b36-b083-fc9f1398eb7b",
 			Name:        "policy1",
 			Description: "policy1",
-			Rules:       `node_prefix "" { policy = "read" }`,
+			Rules:       `node_prefix "" { policy = "read", policy = "read", },`,
 			RaftIndex: RaftIndex{
 				CreateIndex: 1,
 				ModifyIndex: 2,
@@ -413,7 +413,7 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "b35541f0-a88a-48da-bc66-43553c60b628",
 			Name:        "policy2",
 			Description: "policy2",
-			Rules:       `agent_prefix "" { policy = "read" }`,
+			Rules:       `agent_prefix "" { policy = "read" } `,
 			RaftIndex: RaftIndex{
 				CreateIndex: 3,
 				ModifyIndex: 4,
@@ -433,7 +433,8 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "8bf38965-95e5-4e86-9be7-f6070cc0708b",
 			Name:        "policy4",
 			Description: "policy4",
-			Rules:       `service_prefix "" { policy = "read" }`,
+			//test should still pass even with the duplicate key since its resolving the cache
+			Rules: `service_prefix "" { policy = "read"  policy = "read" }`,
 			RaftIndex: RaftIndex{
 				CreateIndex: 7,
 				ModifyIndex: 8,

go.mod

@@ -68,7 +68,7 @@ require (
 	github.com/hashicorp/go-version v1.2.1
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/hashicorp/hcdiag v0.5.1
-	github.com/hashicorp/hcl v1.0.0
+	github.com/hashicorp/hcl v1.0.1-vault-7
 	github.com/hashicorp/hcl/v2 v2.14.1
 	github.com/hashicorp/hcp-scada-provider v0.2.4
 	github.com/hashicorp/hcp-sdk-go v0.80.0
@@ -186,7 +186,7 @@ require (
 	github.com/go-openapi/validate v0.22.4 // indirect
 	github.com/go-ozzo/ozzo-validation v3.6.0+incompatible // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
-	github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
+	github.com/golang-jwt/jwt/v4 v4.5.1 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/golang/snappy v0.0.4 // indirect

go.sum

@@ -287,8 +287,8 @@ github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69
 github.com/goji/httpauth v0.0.0-20160601135302-2da839ab0f4d/go.mod h1:nnjvkQ9ptGaCkuDUx6wNykzzlUixGxvkme+H/lnzb+A=
 github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
 github.com/golang-jwt/jwt/v4 v4.2.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
-github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
+github.com/golang-jwt/jwt/v4 v4.5.1 h1:JdqV9zKUdtaa9gdPlywC3aeoEsR681PlKC+4F5gQgeo=
-github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
+github.com/golang-jwt/jwt/v4 v4.5.1/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/golang/glog v1.1.0 h1:/d3pCKDPWNnvIWe0vVUpNP32qc8U3PDVxySP/y360qE=
 github.com/golang/glog v1.1.0/go.mod h1:pfYeQZ3JWZoXTV5sFc986z3HTpwQs9At6P4ImfuP3NQ=
@@ -488,8 +488,9 @@ github.com/hashicorp/golang-lru/v2 v2.0.0 h1:Lf+9eD8m5pncvHAOCQj49GSN6aQI8XGfI5O
 github.com/hashicorp/golang-lru/v2 v2.0.0/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/hashicorp/hcdiag v0.5.1 h1:KZcx9xzRfEOQ2OMbwPxVvHyXwLLRqYpSHxCEOtHfQ6w=
 github.com/hashicorp/hcdiag v0.5.1/go.mod h1:RMC2KkffN9uJ+5mFSaL67ZFVj4CDeetPF2d/53XpwXo=
-github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
 github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
+github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
+github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
 github.com/hashicorp/hcl/v2 v2.14.1 h1:x0BpjfZ+CYdbiz+8yZTQ+gdLO7IXvOut7Da+XJayx34=
 github.com/hashicorp/hcl/v2 v2.14.1/go.mod h1:e4z5nxYlWNPdDSNYX+ph14EvWYMFm3eP0zIUqPc2jr0=
 github.com/hashicorp/hcp-scada-provider v0.2.4 h1:XvctVEd4VqWVlqN1VA4vIhJANstZrc4gd2oCfrFLWZc=

internal/resource/filter.go

@@ -1,109 +0,0 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: BUSL-1.1
-
-package resource
-
-import (
-	"fmt"
-
-	"github.com/hashicorp/go-bexpr"
-
-	"github.com/hashicorp/consul/proto-public/pbresource"
-)
-
-type MetadataFilterableResources interface {
-	GetMetadata() map[string]string
-}
-
-// FilterResourcesByMetadata will use the provided go-bexpr based filter to
-// retain matching items from the provided slice.
-//
-// The only variables usable in the expressions are the metadata keys prefixed
-// by "metadata."
-//
-// If no filter is provided, then this does nothing and returns the input.
-func FilterResourcesByMetadata[T MetadataFilterableResources](resources []T, filter string) ([]T, error) {
-	if filter == "" || len(resources) == 0 {
-		return resources, nil
-	}
-
-	eval, err := createMetadataFilterEvaluator(filter)
-	if err != nil {
-		return nil, err
-	}
-
-	filtered := make([]T, 0, len(resources))
-	for _, res := range resources {
-		vars := &metadataFilterFieldDetails{
-			Meta: res.GetMetadata(),
-		}
-		match, err := eval.Evaluate(vars)
-		if err != nil {
-			return nil, err
-		}
-		if match {
-			filtered = append(filtered, res)
-		}
-	}
-	if len(filtered) == 0 {
-		return nil, nil
-	}
-	return filtered, nil
-}
-
-// FilterMatchesResourceMetadata will use the provided go-bexpr based filter to
-// determine if the provided resource matches.
-//
-// The only variables usable in the expressions are the metadata keys prefixed
-// by "metadata."
-//
-// If no filter is provided, then this returns true.
-func FilterMatchesResourceMetadata(res *pbresource.Resource, filter string) (bool, error) {
-	if res == nil {
-		return false, nil
-	} else if filter == "" {
-		return true, nil
-	}
-
-	eval, err := createMetadataFilterEvaluator(filter)
-	if err != nil {
-		return false, err
-	}
-
-	vars := &metadataFilterFieldDetails{
-		Meta: res.Metadata,
-	}
-	match, err := eval.Evaluate(vars)
-	if err != nil {
-		return false, err
-	}
-	return match, nil
-}
-
-// ValidateMetadataFilter will validate that the provided filter is going to be
-// a valid input to the FilterResourcesByMetadata function.
-//
-// This is best called from a Validate hook.
-func ValidateMetadataFilter(filter string) error {
-	if filter == "" {
-		return nil
-	}
-
-	_, err := createMetadataFilterEvaluator(filter)
-	return err
-}
-
-func createMetadataFilterEvaluator(filter string) (*bexpr.Evaluator, error) {
-	sampleVars := &metadataFilterFieldDetails{
-		Meta: make(map[string]string),
-	}
-	eval, err := bexpr.CreateEvaluatorForType(filter, nil, sampleVars)
-	if err != nil {
-		return nil, fmt.Errorf("filter %q is invalid: %w", filter, err)
-	}
-	return eval, nil
-}
-
-type metadataFilterFieldDetails struct {
-	Meta map[string]string `bexpr:"metadata"`
-}

internal/resource/filter_test.go

@@ -1,195 +0,0 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: BUSL-1.1
-
-package resource
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
-
-	"github.com/hashicorp/consul/proto-public/pbresource"
-	"github.com/hashicorp/consul/proto/private/prototest"
-	"github.com/hashicorp/consul/sdk/testutil"
-)
-
-func TestFilterResourcesByMetadata(t *testing.T) {
-	type testcase struct {
-		in        []*pbresource.Resource
-		filter    string
-		expect    []*pbresource.Resource
-		expectErr string
-	}
-
-	create := func(name string, kvs ...string) *pbresource.Resource {
-		require.True(t, len(kvs)%2 == 0)
-
-		meta := make(map[string]string)
-		for i := 0; i < len(kvs); i += 2 {
-			meta[kvs[i]] = kvs[i+1]
-		}
-
-		return &pbresource.Resource{
-			Id: &pbresource.ID{
-				Name: name,
-			},
-			Metadata: meta,
-		}
-	}
-
-	run := func(t *testing.T, tc testcase) {
-		got, err := FilterResourcesByMetadata(tc.in, tc.filter)
-		if tc.expectErr != "" {
-			require.Error(t, err)
-			testutil.RequireErrorContains(t, err, tc.expectErr)
-		} else {
-			require.NoError(t, err)
-			prototest.AssertDeepEqual(t, tc.expect, got)
-		}
-	}
-
-	cases := map[string]testcase{
-		"nil input": {},
-		"no filter": {
-			in: []*pbresource.Resource{
-				create("one"),
-				create("two"),
-				create("three"),
-				create("four"),
-			},
-			filter: "",
-			expect: []*pbresource.Resource{
-				create("one"),
-				create("two"),
-				create("three"),
-				create("four"),
-			},
-		},
-		"bad filter": {
-			in: []*pbresource.Resource{
-				create("one"),
-				create("two"),
-				create("three"),
-				create("four"),
-			},
-			filter:    "garbage.value == zzz",
-			expectErr: `Selector "garbage" is not valid`,
-		},
-		"filter everything out": {
-			in: []*pbresource.Resource{
-				create("one"),
-				create("two"),
-				create("three"),
-				create("four"),
-			},
-			filter: "metadata.foo == bar",
-		},
-		"filter simply": {
-			in: []*pbresource.Resource{
-				create("one", "foo", "bar"),
-				create("two", "foo", "baz"),
-				create("three", "zim", "gir"),
-				create("four", "zim", "gaz", "foo", "bar"),
-			},
-			filter: "metadata.foo == bar",
-			expect: []*pbresource.Resource{
-				create("one", "foo", "bar"),
-				create("four", "zim", "gaz", "foo", "bar"),
-			},
-		},
-		"filter prefix": {
-			in: []*pbresource.Resource{
-				create("one", "foo", "bar"),
-				create("two", "foo", "baz"),
-				create("three", "zim", "gir"),
-				create("four", "zim", "gaz", "foo", "bar"),
-				create("four", "zim", "zzz"),
-			},
-			filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
-			expect: []*pbresource.Resource{
-				create("three", "zim", "gir"),
-				create("four", "zim", "gaz", "foo", "bar"),
-			},
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			run(t, tc)
-		})
-	}
-}
-
-func TestFilterMatchesResourceMetadata(t *testing.T) {
-	type testcase struct {
-		res       *pbresource.Resource
-		filter    string
-		expect    bool
-		expectErr string
-	}
-
-	create := func(name string, kvs ...string) *pbresource.Resource {
-		require.True(t, len(kvs)%2 == 0)
-
-		meta := make(map[string]string)
-		for i := 0; i < len(kvs); i += 2 {
-			meta[kvs[i]] = kvs[i+1]
-		}
-
-		return &pbresource.Resource{
-			Id: &pbresource.ID{
-				Name: name,
-			},
-			Metadata: meta,
-		}
-	}
-
-	run := func(t *testing.T, tc testcase) {
-		got, err := FilterMatchesResourceMetadata(tc.res, tc.filter)
-		if tc.expectErr != "" {
-			require.Error(t, err)
-			testutil.RequireErrorContains(t, err, tc.expectErr)
-		} else {
-			require.NoError(t, err)
-			require.Equal(t, tc.expect, got)
-		}
-	}
-
-	cases := map[string]testcase{
-		"nil input": {},
-		"no filter": {
-			res:    create("one"),
-			filter: "",
-			expect: true,
-		},
-		"bad filter": {
-			res:       create("one"),
-			filter:    "garbage.value == zzz",
-			expectErr: `Selector "garbage" is not valid`,
-		},
-		"no match": {
-			res:    create("one"),
-			filter: "metadata.foo == bar",
-		},
-		"match simply": {
-			res:    create("one", "foo", "bar"),
-			filter: "metadata.foo == bar",
-			expect: true,
-		},
-		"match via prefix": {
-			res:    create("four", "zim", "gaz", "foo", "bar"),
-			filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
-			expect: true,
-		},
-		"no match via prefix": {
-			res:    create("four", "zim", "zzz", "foo", "bar"),
-			filter: "(zim in metadata) and (metadata.zim matches `^g.`)",
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			run(t, tc)
-		})
-	}
-}

test-integ/go.mod

@@ -64,7 +64,7 @@ require (
 	github.com/hashicorp/go-uuid v1.0.3 // indirect
 	github.com/hashicorp/go-version v1.2.1 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect
-	github.com/hashicorp/hcl v1.0.0 // indirect
+	github.com/hashicorp/hcl v1.0.1-vault-7 // indirect
 	github.com/hashicorp/hcl/v2 v2.16.2 // indirect
 	github.com/hashicorp/memberlist v0.5.0 // indirect
 	github.com/hashicorp/serf v0.10.1 // indirect

test-integ/go.sum

@@ -141,8 +141,8 @@ github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
-github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
+github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
-github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
+github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
 github.com/hashicorp/hcl/v2 v2.16.2 h1:mpkHZh/Tv+xet3sy3F9Ld4FyI2tUpWe9x3XtPx9f1a0=
 github.com/hashicorp/hcl/v2 v2.16.2/go.mod h1:JRmR89jycNkrrqnMmvPDMd56n1rQJ2Q6KocSLCMCXng=
 github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=

test/integration/consul-container/go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-uuid v1.0.3
 	github.com/hashicorp/go-version v1.2.1
-	github.com/hashicorp/hcl v1.0.0
+	github.com/hashicorp/hcl v1.0.1-vault-7
 	github.com/hashicorp/serf v0.10.1
 	github.com/itchyny/gojq v0.12.12
 	github.com/mitchellh/copystructure v1.2.0

test/integration/consul-container/go.sum

@@ -150,8 +150,8 @@ github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
-github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4=
+github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
-github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=
+github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
 github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=
 github.com/hashicorp/mdns v1.0.4/go.mod h1:mtBihi+LeNXGtG8L9dX59gAEa12BDtBQSp4v/YAJqrc=
 github.com/hashicorp/memberlist v0.5.0 h1:EtYPN8DpAURiapus508I4n9CzHs2W+8NZGbmmR/prTM=