Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe variable in user-function but not in builtin function #471

Closed
timothyhinrichs opened this issue Oct 5, 2017 · 1 comment
Closed
Labels

Comments

@timothyhinrichs
Copy link
Member

timothyhinrichs commented Oct 5, 2017

The safety rules are different for user functions and builtin functions, but there's no clear reason for them to be different. In particular, user functions cannot use iteration variables in arguments, but builtin functions can.

Below we see that z[_] is allowed as the first argument to builtin function concat, but it is not allowed as the first argument to the user function f.

> x = {y | z = ["a", "b", "c"]; concat(z[_], ["123", "456"], y)}
>
> f (x) = x { true }
> x = {y | z = ["a", "b", "c"]; f(z[_], y)}
1 error occurred: 1:1: rego_unsafe_var_error: var _ is unsafe
@tsandall tsandall added the bug label Oct 9, 2017
@tsandall
Copy link
Member

tsandall commented Oct 9, 2017

There's a related issue in the safety check...

If a function input is unsafe, the output vars are added to the safe set regardless. This means the following query f(x, x) would be considered safe (and result in an error during evaluation.)

tsandall added a commit to tsandall/opa that referenced this issue Oct 10, 2017
Previously, functions were implemented with a separate set of types that
had their own code paths in the compiler, eval, etc. These changes
refactor the function implementation so that functions are implemented
as rules with one or more arguments.

By representing functions as rules, we can avoid special casing required
to support functions, e.g., during parse and compile there are a number
of steps that required special casing for functions:

- Parser needed separate grammar definitions for functions (which
  prevented them from being chained or using else)

- Compiler needed separate resolver and type checker implementations
  which was a source of bugs.

In some cases, special casing is unavoidable for now (e.g., during eval)
however this could be improved in the future.

Fixes open-policy-agent#471
Fixes open-policy-agent#467
Fixes open-policy-agent#463
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants