Skip to content

Commit

Permalink
Remove ast.all_refs (#1076)
Browse files Browse the repository at this point in the history
This held a copy of `ast.found.refs` + imported refs
Which was quite redundant. Rewrote all consumers to
go for `ast.found.refs` directly, and imports where
needed.

Also added `util.intesecets` for a common pattern used
in these rules.

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert authored Sep 9, 2024
1 parent 2690748 commit 2e07303
Show file tree
Hide file tree
Showing 11 changed files with 69 additions and 95 deletions.
9 changes: 0 additions & 9 deletions bundle/regal/ast/ast.rego
Original file line number Diff line number Diff line change
Expand Up @@ -181,15 +181,6 @@ _exclude_arg(_, _, arg) if arg.type == "call"
# ignore here, as it's covered elsewhere
_exclude_arg("assign", 0, _)

# METADATA
# description: |
# set containing all references found in the input AST
# NOTE: likely to be deprecated — prefer to use `ast.found.refs` over this
# scope: document
all_refs contains found.refs[_][_]

all_refs contains imported.path if some imported in input.imports

# METADATA
# description: returns the "path" string of any given ref value
ref_to_string(ref) := concat(".", [_ref_part_to_string(i, part) | some i, part in ref])
Expand Down
22 changes: 0 additions & 22 deletions bundle/regal/ast/ast_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import rego.v1

import data.regal.ast
import data.regal.capabilities
import data.regal.util

# regal ignore:rule-length
test_find_vars if {
Expand Down Expand Up @@ -263,27 +262,6 @@ test_find_some_decl_names_in_scope if {

var_names(vars) := {var.value | some var in vars}

test_all_refs if {
policy := `package policy
import data.foo.bar
allow := data.foo.baz
deny[message] {
message := data.foo.bax
}
`

module := regal.parse_module("p.rego", policy)

r := ast.all_refs with input as module

text_refs := {base64.decode(util.to_location_object(ref.location).text) | some ref in r}

text_refs == {":=", "data.foo.bar", "data.foo.bax", "data.foo.baz"}
}

test_provided_capabilities_never_undefined if {
capabilities.provided == {} with data.internal as {}
}
Expand Down
14 changes: 4 additions & 10 deletions bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,21 @@ import rego.v1
import data.regal.ast
import data.regal.config
import data.regal.result
import data.regal.util

report contains violation if {
deprecated_builtins := {
"any", "all", "re_match", "net.cidr_overlap", "set_diff", "cast_array",
"cast_set", "cast_string", "cast_boolean", "cast_null", "cast_object",
}

# if none of the deprecated built-ins are in the
# capabilities for the target, bail out early
any_deprecated_builtin(object.keys(config.capabilities.builtins), deprecated_builtins)

some ref in ast.all_refs
# bail out early if no the deprecated built-ins are in capabilities
util.intersects(object.keys(config.capabilities.builtins), deprecated_builtins)

ref := ast.found.refs[_][_]
call := ref[0]

ast.ref_to_string(call.value) in deprecated_builtins

violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(call))
}

any_deprecated_builtin(caps_builtins, deprecated_builtins) if {
some builtin in caps_builtins
builtin in deprecated_builtins
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,17 @@ import data.regal.ast
import data.regal.result

report contains violation if {
some ref in ast.all_refs
ref := ast.found.refs[_][_]

contains(ast.ref_to_string(ref.value), "._")

violation := result.fail(rego.metadata.chain(), result.location(ref))
}

report contains violation if {
some imported in input.imports

contains(ast.ref_to_string(imported.path.value), "._")

violation := result.fail(rego.metadata.chain(), result.location(imported.path.value))
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,15 @@ import rego.v1
import data.regal.ast
import data.regal.config
import data.regal.result

cfg := config.for_rule("custom", "forbidden-function-call")

any_forbidden_function_called if {
some function in cfg["forbidden-functions"]
function in ast.builtin_functions_called
}
import data.regal.util

report contains violation if {
# avoid traversal if no forbidden function is called
any_forbidden_function_called
cfg := config.for_rule("custom", "forbidden-function-call")

some ref in ast.all_refs
# avoid traversal if no forbidden function is called
util.intersects(util.to_set(cfg["forbidden-functions"]), ast.builtin_functions_called)

ref := ast.found.refs[_][_]
name := ast.ref_to_string(ref[0].value)
name in cfg["forbidden-functions"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,34 @@ import data.regal.ast
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)

value := ast.found.refs[_][_]

value[0].value[0].type == "var"
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)
row := input.regal.file.lines[loc.row - 1]
chr := substring(row, loc.col - 1, 1)

chr == `"`

violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(value[pos]))
}

# Mapping of regex.* functions and the position(s)
# of their "pattern" attributes
re_pattern_functions := {
_re_pattern_functions := {
"find_all_string_submatch_n": [1],
"find_n": [1],
"globs_match": [1, 2],
Expand All @@ -21,7 +46,7 @@ re_pattern_functions := {
"template_match": [1],
}

re_pattern_function_names := {
_re_pattern_function_names := {
"regex.find_all_string_submatch_n",
"regex.find_n",
"regex.globs_match",
Expand All @@ -31,33 +56,3 @@ re_pattern_function_names := {
"regex.split",
"regex.template_match",
}

any_regex_function_called if {
some name in re_pattern_function_names
name in ast.builtin_functions_called
}

report contains violation if {
# skip expensive walk if no builtin regex function calls are registered
any_regex_function_called

some value in ast.all_refs

value[0].value[0].type == "var"
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)
row := input.regal.file.lines[loc.row - 1]
chr := substring(row, loc.col - 1, 1)

chr == `"`

violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(value[pos]))
}
13 changes: 12 additions & 1 deletion bundle/regal/rules/imports/circular-import/circular_import.rego
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import data.regal.result
import data.regal.util

refs contains ref if {
some r in ast.all_refs
r := ast.found.refs[_][_]

r.value[0].value == "data"

Expand All @@ -26,6 +26,17 @@ refs contains ref if {
}
}

refs contains ref if {
some imported in ast.imports

imported.path.value[0].value == "data"

ref := {
"package_path": concat(".", [e.value | some e in imported.path.value]),
"location": object.remove(util.to_location_object(imported.path.location), {"text"}),
}
}

aggregate contains entry if {
count(refs) > 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import data.regal.ast
import data.regal.result

report contains violation if {
some value in ast.all_refs
value := ast.found.refs[_][_]

value[0].value[0].type == "var"
value[0].value[0].value in {"equal", "neq"} # perhaps add more operators here?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import data.regal.ast
import data.regal.result

report contains violation if {
# skip expensive walk operation if no print calls are registered
# skip traversing refs if no print calls are registered
"print" in ast.builtin_functions_called

some value in ast.all_refs
value := ast.found.refs[_][_]

value[0].value[0].type == "var"
value[0].value[0].value == "print"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,13 @@ import rego.v1

import data.regal.ast
import data.regal.result

print_or_trace_called if {
some name in {"print", "trace"}
name in ast.builtin_functions_called
}
import data.regal.util

report contains violation if {
# skip iteration of refs if no print or trace calls are registered
print_or_trace_called
util.intersects(ast.builtin_functions_called, {"print", "trace"})

some ref in ast.all_refs
ref := ast.found.refs[_][_]

ref[0].value[0].type == "var"
ref[0].value[0].value in {"print", "trace"}
Expand Down
6 changes: 6 additions & 0 deletions bundle/regal/util/util.rego
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,9 @@ json_pretty(value) := json.marshal_with_options(value, {
})

rest(arr) := array.slice(arr, 1, count(arr))

to_set(x) := x if is_set(x)

to_set(x) := {y | some y in x} if not is_set(x)

intersects(s1, s2) if count(intersection({s1, s2})) > 0

0 comments on commit 2e07303

Please sign in to comment.