bugs/not-equals-in-loop

bundle/regal/rules/bugs/bugs.rego

@@ -120,6 +120,34 @@ report contains violation if {
 	violation := result.fail(rego.metadata.rule(), result.location(expr.terms[0]))
 }
 
+# METADATA
+# title: not-equals-in-loop
+# description: Use of != in loop
+# related_resources:
+# - description: documentation
+#   ref: $baseUrl/$category/not-equals-in-loop
+# custom:
+#   category: bugs
+report contains violation if {
+	config.for_rule(rego.metadata.rule()).level != "ignore"
+
+	some rule in input.rules
+	some expr in rule.body
+
+	expr.terms[0].type == "ref"
+	expr.terms[0].value[0].type == "var"
+	expr.terms[0].value[0].value == "neq"
+
+	some neq_term in array.slice(expr.terms, 1, count(expr.terms))
+	neq_term.type == "ref"
+
+	some i
+	neq_term.value[i].type == "var"
+	startswith(neq_term.value[i].value, "$")
+
+	violation := result.fail(rego.metadata.rule(), result.location(expr.terms[0]))
+}
+
 # regal ignore:external-reference
 illegal_value_ref(value) if not value in _rule_names
 

bundle/regal/rules/bugs/bugs_test.rego

@@ -113,6 +113,52 @@ test_success_return_value_assigned if {
 	report(`allow { x := indexof("s", "s") }`) == set()
 }
 
+test_fail_neq_in_loop if {
+	r := report(`deny {
+		"admin" != input.user.groups[_]
+		input.user.groups[_] != "admin"
+	}`)
+	r == {
+		{
+			"category": "bugs",
+			"description": "Use of != in loop",
+			"level": "error",
+			"location": {"col": 11, "file": "policy.rego", "row": 9, "text": "\t\t\"admin\" != input.user.groups[_]"},
+			"related_resources": [{
+				"description": "documentation",
+				"ref": "https://github.com/StyraInc/regal/blob/main/docs/rules/bugs/not-equals-in-loop.md",
+			}],
+			"title": "not-equals-in-loop",
+		},
+		{
+			"category": "bugs",
+			"description": "Use of != in loop",
+			"level": "error",
+			"location": {"col": 24, "file": "policy.rego", "row": 10, "text": "\t\tinput.user.groups[_] != \"admin\""},
+			"related_resources": [{
+				"description": "documentation",
+				"ref": "https://github.com/StyraInc/regal/blob/main/docs/rules/bugs/not-equals-in-loop.md",
+			}],
+			"title": "not-equals-in-loop",
+		},
+	}
+}
+
+test_fail_neq_in_loop_one_liner if {
+	r := report(`deny if "admin" != input.user.groups[_]`)
+	r == {{
+		"category": "bugs",
+		"description": "Use of != in loop",
+		"level": "error",
+		"location": {"col": 17, "file": "policy.rego", "row": 8, "text": "deny if \"admin\" != input.user.groups[_]"},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": "https://github.com/StyraInc/regal/blob/main/docs/rules/bugs/not-equals-in-loop.md",
+		}],
+		"title": "not-equals-in-loop",
+	}}
+}
+
 report(snippet) := report if {
 	# regal ignore:external-reference
 	report := bugs.report with input as ast.with_future_keywords(snippet)

docs/rules/bugs/not-equals-in-loop.md

@@ -0,0 +1,69 @@
+# not-equals-in-loop
+
+**Summary**: Use of != in loop
+
+**Category**: Bugs
+
+**Avoid**
+```rego
+package policy
+
+import future.keywords.if
+
+deny if {
+    "admin" != input.user.roles[_]
+}
+```
+
+**Prefer**
+```rego
+package policy
+
+import future.keywords.if
+import future.keywords.in
+
+deny if {
+    not "admin" in input.user.roles
+}
+
+# Or as a one-liner
+deny if not "admin" in input.user.roles
+```
+
+## Rationale
+
+Likely one of the most common mistakes in Rego is to use `!=` in a loop thinking it means "not in". It took some years
+for the `in` keyword to be added to Rego, so perhaps it's not surprising that this mistake is a common one even to this
+day. If it doesn't mean "not in", what does it mean?
+
+```rego
+package policy
+
+import future.keywords.if
+
+deny if {
+    "admin" != input.user.roles[_]
+}
+```
+
+The body of the `deny` rule above roughly translates to "for any item in `input.user.roles`, return true if the item is
+not `admin`". This is almost never what the policy author intended. What the policy author likely intended was 
+"deny if `admin` is not in `input.user.roles`". The above policy would thus **not** deny a user with the roles
+`["user", "admin"]` since the first item in the array is not "admin". This is almost never what the policy author
+intended.
+
+**Note**: This linter rule currently only checks for `!=` in a non-nested comparison where iteration happens on either
+side of the comparison in the same expression. This will be improved in time. Another limitation is that this rule
+currently only checks for wildcard iteration (`[_]`).
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules: 
+  bugs:
+    not-equals-in-loop:
+      # one of "error", "warning", "ignore"
+      level: error
+```