Rule: ambiguous-scope (#795)

Fixes #778

Signed-off-by: Anders Eknert <[email protected]>

README.md

@@ -203,6 +203,7 @@ The following rules are currently available:
 | custom      | [naming-convention](https://docs.styra.com/regal/rules/custom/naming-convention)                      | Naming convention violation                               |
 | custom      | [one-liner-rule](https://docs.styra.com/regal/rules/custom/one-liner-rule)                            | Rule body could be made a one-liner                       |
 | custom      | [prefer-value-in-head](https://docs.styra.com/regal/rules/custom/prefer-value-in-head)                | Prefer value in rule head                                 |
+| idiomatic   | [ambiguous-scope](https://docs.styra.com/regal/rules/idiomatic/ambiguous-scope)                       | Ambiguous metadata scope                                  |
 | idiomatic   | [boolean-assignment](https://docs.styra.com/regal/rules/idiomatic/boolean-assignment)                 | Prefer `if` over boolean assignment                       |
 | idiomatic   | [custom-has-key-construct](https://docs.styra.com/regal/rules/idiomatic/custom-has-key-construct)     | Custom function may be replaced by `in` and `object.keys` |
 | idiomatic   | [custom-in-construct](https://docs.styra.com/regal/rules/idiomatic/custom-in-construct)               | Custom function may be replaced by `in` keyword           |

bundle/regal/ast/ast.rego

@@ -343,6 +343,7 @@ all_rules_refs contains value if {
 # METADATA
 # title: all_refs
 # description: set containing all references found in the input AST
+# scope: document
 all_refs contains value if some value in all_rules_refs
 
 all_refs contains value if {
@@ -377,6 +378,7 @@ static_rule_ref(ref) if every t in array.slice(ref, 1, count(ref)) {
 # description: |
 #   return the name of a rule if, and only if it only has static parts with
 #   no vars. This could be "username", or "user.name", but not "user[name]"
+# scope: document
 static_rule_name(rule) := rule.head.ref[0].value if count(rule.head.ref) == 1
 
 static_rule_name(rule) := concat(".", array.concat([rule.head.ref[0].value], [ref.value |
@@ -438,6 +440,7 @@ function_ret_in_args(fn_name, terms) if {
 # METADATA
 # description: |
 #   answers if provided rule is implicitly assigned boolean true, i.e. allow { .. } or not
+# scope: document
 implicit_boolean_assignment(rule) if {
 	# note the missing location attribute here, which is how we distinguish
 	# between implicit and explicit assignments

bundle/regal/ast/imports.rego

@@ -8,6 +8,7 @@ default imports := []
 # description: |
 #   same as input.imports but with a default value (`[]`), making
 #   it safe to refer to without risk of halting evaluation
+# scope: document
 imports := input.imports
 
 # METADATA

bundle/regal/capabilities.rego

@@ -11,6 +11,7 @@ default provided := {}
 #   The capabilities object for Regal itself. Use `config.capabilities`
 #   to get the capabilities for the target environment (i.e. the policies
 #   getting linted).
+# scope: document
 provided := data.internal.capabilities
 
 has_object_keys if "object.keys" in object.keys(config.capabilities.builtins)

bundle/regal/config/config.rego

@@ -20,6 +20,7 @@ default for_rule(_, _) := {"level": "error"}
 #   Returns the configuration applied (i.e. the provided configuration
 #   merged with any user configuration and possibly command line overrides)
 #   to the rule matching the category and title.
+# scope: document
 for_rule(category, title) := _with_level(category, title, "ignore") if {
 	force_disabled(category, title)
 } else := _with_level(category, title, "error") if {

bundle/regal/config/provided/data.yaml

@@ -44,6 +44,8 @@ rules:
     prefer-value-in-head:
       level: ignore
   idiomatic:
+    ambiguous-scope:
+      level: error
     boolean-assignment:
       level: error
     custom-has-key-construct:

bundle/regal/rules/idiomatic/ambiguous_scope.rego

@@ -0,0 +1,57 @@
+# METADATA
+# description: Ambiguous metadata scope
+package regal.rules.idiomatic["ambiguous-scope"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.result
+import data.regal.util
+
+report contains violation if {
+	some name, rules in _incremental_rules
+
+	# should internal rules have metadata at all? not sure, but
+	# let's loosen up the restrictions on them for the moment
+	not startswith(name, "_")
+
+	rules_with_annotations := [rule |
+		some rule in rules
+
+		rule.annotations
+	]
+
+	# a single rule with a single annotation — that means it is also
+	# has scope == rule as if it had a scope == document it would be seen
+	# on each of the rules
+	count(rules_with_annotations) == 1
+	count(rules_with_annotations[0].annotations) == 1
+
+	annotation := rules_with_annotations[0].annotations[0]
+
+	# treat this as an exception for now — it's arguably an issue in itself
+	# that an entrypoint can be scoped to "rule", as clearly that's not how
+	# entrypoints work
+	not annotation.entrypoint
+
+	not _explicit_scope(rules_with_annotations[0], input.regal.file.lines)
+
+	violation := result.fail(rego.metadata.chain(), result.location(annotation))
+}
+
+_incremental_rules[name] contains rule if {
+	some rule in input.rules
+
+	name := ast.ref_to_string(rule.head.ref)
+
+	util.has_duplicates(_rule_names, name)
+}
+
+_rule_names := [ast.ref_to_string(rule.head.ref) | some rule in input.rules]
+
+_explicit_scope(rule, lines) if {
+	some i in numbers.range(rule.annotations[0].location.row - 1, rule.head.location.row - 2)
+	line := lines[i]
+
+	startswith(line, "# scope:")
+}

bundle/regal/rules/idiomatic/ambiguous_scope_test.rego

@@ -0,0 +1,127 @@
+package regal.rules.idiomatic["ambiguous-scope_test"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.config
+
+import data.regal.rules.idiomatic["ambiguous-scope"] as rule
+
+test_ambiguous_scope_lonely_annotated_rule if {
+	module := ast.with_rego_v1(`
+# METADATA
+# title: allow
+allow if input.x
+
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == {{
+		"category": "idiomatic",
+		"description": "Ambiguous metadata scope",
+		"level": "error",
+		"location": {"col": 1, "file": "policy.rego", "row": 6, "text": "# METADATA"},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/ambiguous-scope", "idiomatic"),
+		}],
+		"title": "ambiguous-scope",
+	}}
+}
+
+test_ambiguous_scope_lonely_annotated_function if {
+	module := ast.with_rego_v1(`
+# METADATA
+# title: func
+func(x) if x < 10
+
+func(x) if x < "10"
+	`)
+
+	r := rule.report with input as module
+	r == {{
+		"category": "idiomatic",
+		"description": "Ambiguous metadata scope",
+		"level": "error",
+		"location": {"col": 1, "file": "policy.rego", "row": 6, "text": "# METADATA"},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/ambiguous-scope", "idiomatic"),
+		}],
+		"title": "ambiguous-scope",
+	}}
+}
+
+test_not_ambiguous_scope_all_scoped_to_rule if {
+	module := ast.with_rego_v1(`
+# METADATA
+# description: input has x
+allow if input.x
+
+# METADATA
+# description: input has y
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_not_ambiguous_document_scope if {
+	module := ast.with_rego_v1(`
+# METADATA
+# description: input has x or y
+# scope: document
+allow if input.x
+
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_not_ambiguous_entrypoint_exception if {
+	module := ast.with_rego_v1(`
+# METADATA
+# entrypoint: true
+allow if input.x
+
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_not_ambiguous_both_document_and_rule if {
+	module := ast.with_rego_v1(`
+# METADATA
+# description: input has x or y
+# scope: document
+
+# METADATA
+# description: input has x
+allow if input.x
+
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_not_ambiguous_explicit_rule_scope if {
+	module := ast.with_rego_v1(`
+# METADATA
+# description: input has x
+# scope: rule
+allow if input.x
+
+allow if input.y
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}

bundle/regal/rules/idiomatic/custom_has_key_construct.rego

@@ -26,7 +26,7 @@ report contains violation if {
 	terms[0].value[0].type == "var"
 	terms[0].value[0].value == "eq"
 
-	[_, ref] := normalize_eq_terms(terms)
+	[_, ref] := _normalize_eq_terms(terms)
 
 	ref.value[0].type == "var"
 	ref.value[0].value in arg_names
@@ -36,15 +36,14 @@ report contains violation if {
 	violation := result.fail(rego.metadata.chain(), result.location(rule.head))
 }
 
-# METADATA
-# description: Normalize var to always always be on the left hand side
-normalize_eq_terms(terms) := [terms[1], terms[2]] if {
+# normalize var to always always be on the left hand side
+_normalize_eq_terms(terms) := [terms[1], terms[2]] if {
 	terms[1].type == "var"
 	startswith(terms[1].value, "$")
 	terms[2].type == "ref"
 }
 
-normalize_eq_terms(terms) := [terms[2], terms[1]] if {
+_normalize_eq_terms(terms) := [terms[2], terms[1]] if {
 	terms[1].type == "ref"
 	terms[2].type == "var"
 	startswith(terms[2].value, "$")

bundle/regal/rules/idiomatic/custom_in_construct.rego

@@ -21,7 +21,7 @@ report contains violation if {
 	terms[0].value[0].type == "var"
 	terms[0].value[0].value in {"eq", "equal"}
 
-	[var, ref] := normalize_eq_terms(terms)
+	[var, ref] := _normalize_eq_terms(terms)
 
 	var.value in arg_names
 	ref.value[0].value in arg_names
@@ -31,15 +31,14 @@ report contains violation if {
 	violation := result.fail(rego.metadata.chain(), result.location(rule.head))
 }
 
-# METADATA
-# description: Normalize var to always always be on the left hand side
-normalize_eq_terms(terms) := [terms[1], terms[2]] if {
+# normalize var to always always be on the left hand side
+_normalize_eq_terms(terms) := [terms[1], terms[2]] if {
 	terms[1].type == "var"
 	terms[2].type == "ref"
 	terms[2].value[0].type == "var"
 }
 
-normalize_eq_terms(terms) := [terms[2], terms[1]] if {
+_normalize_eq_terms(terms) := [terms[2], terms[1]] if {
 	terms[1].type == "ref"
 	terms[1].value[0].type == "var"
 	terms[2].type == "var"

bundle/regal/rules/idiomatic/equals_pattern_matching.rego

@@ -30,7 +30,7 @@ report contains violation if {
 	val.value[0].value[0].type == "var"
 	val.value[0].value[0].value == "equal"
 
-	terms := normalize_eq_terms(val.value, ast.scalar_types)
+	terms := _normalize_eq_terms(val.value, ast.scalar_types)
 	terms[0].value in arg_var_names
 
 	violation := result.fail(rego.metadata.chain(), result.location(fn))
@@ -63,21 +63,20 @@ report contains violation if {
 	expr.terms[0].value[0].type == "var"
 	expr.terms[0].value[0].value == "equal"
 
-	terms := normalize_eq_terms(expr.terms, ast.scalar_types)
+	terms := _normalize_eq_terms(expr.terms, ast.scalar_types)
 	terms[0].value in arg_var_names
 
 	violation := result.fail(rego.metadata.chain(), result.location(fn))
 }
 
-# METADATA
-# description: Normalize var to always always be on the left hand side
-normalize_eq_terms(terms, scalar_types) := [terms[1], terms[2]] if {
+# normalize var to always always be on the left hand side
+_normalize_eq_terms(terms, scalar_types) := [terms[1], terms[2]] if {
 	terms[1].type == "var"
 	not startswith(terms[1].value, "$")
 	terms[2].type in scalar_types
 }
 
-normalize_eq_terms(terms, scalar_types) := [terms[2], terms[1]] if {
+_normalize_eq_terms(terms, scalar_types) := [terms[2], terms[1]] if {
 	terms[1].type in scalar_types
 	terms[2].type == "var"
 	not startswith(terms[2].value, "$")

bundle/regal/util/util.rego

@@ -1,7 +1,11 @@
+# METADATA
+# description: various utility functions for linter policies
 package regal.util
 
 import rego.v1
 
+# METADATA
+# description: returns true if string is snake_case formatted
 is_snake_case(str) if str == lower(str)
 
 # METADATA
@@ -19,6 +23,13 @@ find_duplicates(arr) := {indices |
 	count(indices) > 1
 }
 
+# METADATA
+# description: returns true if array has duplicates of item
+has_duplicates(array, item) if count([x |
+	some x in array
+	x == item
+]) > 1
+
 # METADATA
 # description: |
 #   returns an array of arrays built from all parts of the provided path array,