Rule: invalid-regexp (#1937)

Simple, but a nice addition!

Fixes #1933

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

Author: anderseknert

bundle/regal/ast/regexp.rego

@@ -0,0 +1,49 @@
+package regal.ast
+
+import data.regal.util
+
+# METADATA
+# description: All regular expression 'literals' (represented as strings) found in the AST
+regexp.found.literals contains term if {
+	# skip traversing refs if no builtin regex function calls are registered
+	util.intersects(regexp.pattern_function_names, builtin_functions_called)
+
+	value := found.calls[_][_]
+
+	value[0].value[0].value == "regex"
+
+	# The name following "regex.", e.g. "match"
+	name := value[0].value[1].value
+
+	some pos in regexp.pattern_functions[name]
+
+	term := value[pos]
+
+	term.type == "string"
+}
+
+# METADATA
+# description: Mapping of regex.* functions and the position(s) of their "pattern" attributes
+regexp.pattern_functions := {
+	"find_all_string_submatch_n": [1],
+	"find_n": [1],
+	"globs_match": [1, 2],
+	"is_valid": [1],
+	"match": [1],
+	"replace": [2],
+	"split": [1],
+	"template_match": [1],
+}
+
+# METADATA
+# description: Set of all regex.* function names that take a regex pattern as an argument
+regexp.pattern_function_names := {
+	"regex.find_all_string_submatch_n",
+	"regex.find_n",
+	"regex.globs_match",
+	"regex.is_valid",
+	"regex.match",
+	"regex.replace",
+	"regex.split",
+	"regex.template_match",
+}

bundle/regal/config/provided/data.yaml

@@ -28,6 +28,8 @@ rules:
       level: error
     invalid-metadata-attribute:
       level: error
+    invalid-regexp:
+      level: error
     leaked-internal-reference:
       level: error
     not-equals-in-loop:

bundle/regal/rules/bugs/invalid-regexp/invalid_regexp.rego

@@ -0,0 +1,17 @@
+# METADATA
+# description: Invalid regular expression
+# related_resources:
+#   - description: documentation
+#     ref: https://www.openpolicyagent.org/projects/regal/rules/bugs/invalid-regexp
+package regal.rules.bugs["invalid-regexp"]
+
+import data.regal.ast
+import data.regal.result
+
+report contains violation if {
+	some term in ast.regexp.found.literals
+
+	not regex.is_valid(term.value)
+
+	violation := result.fail(rego.metadata.chain(), result.location(term))
+}

bundle/regal/rules/bugs/invalid-regexp/invalid_regexp_test.rego

@@ -0,0 +1,60 @@
+package regal.rules.bugs["invalid-regexp_test"]
+
+import data.regal.ast
+import data.regal.capabilities
+import data.regal.config
+import data.regal.util
+
+import data.regal.rules.bugs["invalid-regexp"] as rule
+
+test_fail_invalid_regexp if {
+	r := rule.report with input as ast.policy("r := regex.match(`(`, input.string)")
+		with config.capabilities as capabilities.provided
+
+	r == {{
+		"category": "bugs",
+		"description": "Invalid regular expression",
+		"level": "error",
+		"location": {
+			"col": 18,
+			"end": {
+				"col": 21,
+				"row": 3,
+			},
+			"file": "policy.rego",
+			"row": 3,
+			"text": "r := regex.match(`(`, input.string)",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": "https://www.openpolicyagent.org/projects/regal/rules/bugs/invalid-regexp",
+		}],
+		"title": "invalid-regexp",
+	}}
+}
+
+test_fail_invalid_regexp_for_pattern_function[call] if {
+	some call in [
+		"regex.find_all_string_submatch_n(`(`, input.x, 1)",
+		"regex.find_n(`(`, input.x, 1)",
+		"regex.globs_match(`(`, `[abc]`)",
+		"regex.globs_match(`[abc]`, `(`)",
+		"regex.is_valid(`(`)",
+		"regex.match(`(`, input.x)",
+		"regex.replace(input.x, `(`, `y`)",
+		"regex.split(`(`, input.x)",
+		"regex.template_match(`(`, input.x, `<`, `>`)",
+	]
+
+	r := rule.report with input as ast.policy($"r := {call}")
+		with config.capabilities as capabilities.provided
+
+	util.single_set_item(r).location.text == $"r := {call}"
+}
+
+test_pass_valid_regexp if {
+	r := rule.report with input as ast.policy("r := regex.match(`[abc]`, input.string)")
+		with config.capabilities as capabilities.provided
+
+	r == set()
+}

bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego

@@ -10,48 +10,12 @@ import data.regal.result
 import data.regal.util
 
 report contains violation if {
-	# skip traversing refs if no builtin regex function calls are registered
-	util.intersects(_re_pattern_function_names, ast.builtin_functions_called)
+	some term in ast.regexp.found.literals
 
-	value := ast.found.calls[_][_]
-
-	value[0].value[0].value == "regex"
-
-	# The name following "regex.", e.g. "match"
-	name := value[0].value[1].value
-
-	some pos in _re_pattern_functions[name]
-
-	value[pos].type == "string"
-
-	loc := util.to_location_object(value[pos].location)
+	loc := util.to_location_object(term.location)
 	row := input.regal.file.lines[loc.row - 1]
 
 	substring(row, loc.col - 1, 1) == `"`
 
-	violation := result.fail(rego.metadata.chain(), result.location(value[pos]))
-}
-
-# Mapping of regex.* functions and the position(s)
-# of their "pattern" attributes
-_re_pattern_functions := {
-	"find_all_string_submatch_n": [1],
-	"find_n": [1],
-	"globs_match": [1, 2],
-	"is_valid": [1],
-	"match": [1],
-	"replace": [2],
-	"split": [1],
-	"template_match": [1],
-}
-
-_re_pattern_function_names := {
-	"regex.find_all_string_submatch_n",
-	"regex.find_n",
-	"regex.globs_match",
-	"regex.is_valid",
-	"regex.match",
-	"regex.replace",
-	"regex.split",
-	"regex.template_match",
+	violation := result.fail(rego.metadata.chain(), result.location(term))
 }

docs/rules/bugs/import-shadows-rule.md

@@ -5,18 +5,20 @@
 **Category**: Bugs
 
 **Avoid**
+
 ```rego
 package policy
 
 import data.resources
 
-# 'resources' shadowed by import 
+# 'resources' shadowed by import
 resources contains resource if {
     # ...
 }
 ```
 
 **Prefer**
+
 ```rego
 package policy
 
@@ -56,3 +58,7 @@ rules:
       # one of "error", "warning", "ignore"
       level: error
 ```
+
+## Related Resources
+
+- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/import-shadows-rule/import_shadows_rule.rego)

docs/rules/bugs/invalid-regexp.md

@@ -0,0 +1,46 @@
+# invalid-regexp
+
+**Summary**: Invalid regular expression
+
+**Category**: Bugs
+
+**Avoid**
+
+```rego
+package policy
+
+invalid if regex.match(`[abc`, input.text)
+```
+
+**Prefer**
+
+```rego
+package policy
+
+valid if regex.match(`[abc]`, input.text)
+```
+
+## Rationale
+
+An invalid regular expression typically fails silently (i.e. the result is undefined) at runtime when OPA evaluates the
+function call, or with a runtime error if the `show-builtin-errors` option is enabled. While hopefully caught by unit
+tests, tracking down a typo in a regular expression is still time consuming. This rule instead analyzes any regular
+expressions found in a policy as you author it (using OPA's own `regex.is_valid` function) and reports invalid patterns
+directly.
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  bugs:
+    invalid-regexp:
+      # one of "error", "warning", "ignore"
+      level: error
+```
+
+## Related Resources
+
+- OPA Docs: [Regex Functions](https://www.openpolicyagent.org/docs/latest/policy-reference/#regex-functions)
+- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/invalid-regexp/invalid_regexp.rego)

docs/rules/bugs/redundant-loop-count.md

@@ -5,6 +5,7 @@
 **Category**: Bugs
 
 **Avoid**
+
 ```rego
 package policy
 
@@ -17,6 +18,7 @@ allow if {
 ```
 
 **Prefer**
+
 ```rego
 package policy
 
@@ -80,3 +82,7 @@ rules:
       # one of "error", "warning", "ignore"
       level: error
 ```
+
+## Related Resources
+
+- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/bugs/redundant-loop-count/redundant_loop_count.rego)

docs/rules/idiomatic/superfluous-object-get.md

@@ -41,3 +41,7 @@ rules:
       # one of "error", "warning", "ignore"
       level: error
 ```
+
+## Related Resources
+
+- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/idiomatic/superfluous-object-get/superfluous_object_get.rego)

docs/rules/imports/confusing-alias.md

@@ -5,6 +5,7 @@
 **Category**: Imports
 
 **Avoid**
+
 ```rego
 package policy
 
@@ -14,6 +15,7 @@ import data.resources.users as employees
 ```
 
 **Prefer**
+
 ```rego
 package policy
 
@@ -50,3 +52,7 @@ rules:
       # one of "error", "warning", "ignore"
       level: error
 ```
+
+## Related Resources
+
+- GitHub: [Source Code](https://github.com/open-policy-agent/regal/blob/main/bundle/regal/rules/imports/confusing-alias/confusing_alias.rego)