use-in-operator: extend check to include static refs (#499)

Fixes #453

Signed-off-by: Anders Eknert <anders@styra.com>

bundle/regal/ast.rego

@@ -310,6 +310,10 @@ _ref_part_to_string(i, ref) := concat("", ["$", ref.value]) if {
 	i > 0
 }
 
+static_ref(ref) if every t in array.slice(ref.value, 1, count(ref.value)) {
+	t.type != "var"
+}
+
 # METADATA
 # description: provides a set of all built-in function calls made in input policy
 builtin_functions_called contains name if {

bundle/regal/rules/bugs/redundant_existence_check.rego

@@ -13,7 +13,7 @@ report contains violation if {
 
 	expr.terms.type == "ref"
 
-	static_ref(expr.terms)
+	ast.static_ref(expr.terms)
 
 	ref_str := ast.ref_to_string(expr.terms.value)
 
@@ -25,9 +25,3 @@ report contains violation if {
 
 	violation := result.fail(rego.metadata.chain(), result.location(expr))
 }
-
-static_ref(ref) if {
-	every t in array.slice(ref.value, 1, count(ref.value)) {
-		t.type == "string"
-	}
-}

bundle/regal/rules/idiomatic/use_in_operator.rego

@@ -4,40 +4,47 @@ package regal.rules.idiomatic["use-in-operator"]
 
 import rego.v1
 
+import data.regal.ast
 import data.regal.result
 
 report contains violation if {
 	some terms in eq_exprs_terms
 
-	terms[1].type in {"array", "boolean", "object", "null", "number", "set", "string", "var"}
-	terms[2].type == "ref"
+	nl_terms := non_loop_term(terms)
+	count(nl_terms) == 1
 
-	last := regal.last(terms[2].value)
+	nlt := nl_terms[0]
+	static_term(nlt.term)
 
-	last.type == "var"
-	startswith(last.value, "$")
-
-	violation := result.fail(rego.metadata.chain(), result.location(terms[2].value[0]))
+	# Use the non-loop term positon to determine the
+	# location of the loop term (3 is the count of terms)
+	violation := result.fail(rego.metadata.chain(), result.location(terms[3 - nlt.pos]))
 }
 
-report contains violation if {
-	some terms in eq_exprs_terms
+eq_exprs_terms contains terms if {
+	terms := input.rules[_].body[_].terms
 
-	terms[1].type == "ref"
-	terms[2].type in {"array", "boolean", "object", "null", "number", "set", "string", "var"}
+	terms[0].type == "ref"
+	terms[0].value[0].type == "var"
+	terms[0].value[0].value == "equal"
+}
 
-	last := regal.last(terms[1].value)
+non_loop_term(terms) := [{"pos": i + 1, "term": term} |
+	some i, term in array.slice(terms, 1, 3)
+	not loop_term(term)
+]
 
+loop_term(term) if {
+	term.type == "ref"
+	term.value[0].type == "var"
+	last := regal.last(term.value)
 	last.type == "var"
 	startswith(last.value, "$")
-
-	violation := result.fail(rego.metadata.chain(), result.location(terms[1].value[0]))
 }
 
-eq_exprs_terms contains terms if {
-	terms := input.rules[_].body[_].terms
+static_term(term) if term.type in {"array", "boolean", "object", "null", "number", "set", "string", "var"}
 
-	terms[0].type == "ref"
-	terms[0].value[0].type == "var"
-	terms[0].value[0].value == "equal"
+static_term(term) if {
+	term.type == "ref"
+	ast.static_ref(term)
 }

bundle/regal/rules/idiomatic/use_in_operator_test.rego

@@ -176,7 +176,41 @@ test_fail_use_in_operator_var_rhs if {
 	}}
 }
 
-test_success_refs_both_sides if {
+test_fail_use_in_operator_string_ref_lhs if {
+	r := rule.report with input as ast.policy(`allow {
+		data.roles.admin == input.user.roles[_]
+	}`)
+	r == {{
+		"category": "idiomatic",
+		"description": "Use in to check for membership",
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/use-in-operator", "idiomatic"),
+		}],
+		"title": "use-in-operator",
+		"location": {"col": 23, "file": "policy.rego", "row": 4, "text": "\t\tdata.roles.admin == input.user.roles[_]"},
+		"level": "error",
+	}}
+}
+
+test_fail_use_in_operator_string_ref_rhs if {
+	r := rule.report with input as ast.policy(`allow {
+		input.user.roles[_] == data.roles.admin
+	}`)
+	r == {{
+		"category": "idiomatic",
+		"description": "Use in to check for membership",
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/use-in-operator", "idiomatic"),
+		}],
+		"title": "use-in-operator",
+		"location": {"col": 3, "file": "policy.rego", "row": 4, "text": "\t\tinput.user.roles[_] == data.roles.admin"},
+		"level": "error",
+	}}
+}
+
+test_success_loop_refs_both_sides if {
 	r := rule.report with input as ast.policy(`allow { required_roles[_] == input.user.roles[_] }`)
 	r == set()
 }