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

Inconsistencies with external-reference warning #719

Closed
asleire opened this issue May 15, 2024 · 2 comments · Fixed by #723
Closed

Inconsistencies with external-reference warning #719

asleire opened this issue May 15, 2024 · 2 comments · Fixed by #723

Comments

@asleire
Copy link

asleire commented May 15, 2024

When linting my code I stumbled upon a case where I expected an external-reference warning, but there was none. I wrote a few test functions that I believe should trigger the warning but it doesn't

See play here: https://play.openpolicyagent.org/p/o5xBt1XUHP

func_3 and func_5 trigger warnings as expected, but the other funcs do not

@anderseknert
Copy link
Member

Thanks @asleire!

Yeah, the implementation of this rule is rather simplistic.. but I'm still surrpised by a few of these. We have a few rules that can't currently catch all violations — typically because it's really expensive to do so in some cases — but we'll always have zero tolerance for false positives.

In this case it looks like we're already doing an expensive walk, so finding more external references currently not reported seems doable.

I'll take a look.

anderseknert added a commit that referenced this issue May 20, 2024
The following cases are now covered:

```rego
f(_) := r

f(_) := {"r": r}
```
And variations thereof.

Fixes #719

Although it doesn't fix the `func_2` example there, this is about as
much we can do without some major effort.. so I'm not sure it's worth
keeping that issue open as I doubt we'll get there anytime soon.

Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert added a commit that referenced this issue May 20, 2024
The following cases are now covered:

```rego
f(_) := r

f(_) := {"r": r}
```
And variations thereof.

Fixes #719

Although it doesn't fix the `func_2` example there, this is about as
much we can do without some major effort.. so I'm not sure it's worth
keeping that issue open as I doubt we'll get there anytime soon.

Signed-off-by: Anders Eknert <anders@styra.com>
@anderseknert
Copy link
Member

anderseknert commented May 20, 2024

PR up to fix every issue reported other than func_2. I looked into that one briefly but it looks like it would require more effort than it's probably worth at this point. Might open another issue later to deal with some remaining items, but this level of coverage feels quite alright to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants