-
Notifications
You must be signed in to change notification settings - Fork 35
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Like a few other rules, this won't catch all violations as we don't yet traverse nested structures like comprehensions or `every` constructs. Fixes #79 Signed-off-by: Anders Eknert <anders@styra.com>
- Loading branch information
1 parent
269484a
commit 99a5101
Showing
4 changed files
with
126 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
# not-equals-in-loop | ||
|
||
**Summary**: Use of != in loop | ||
|
||
**Category**: Bugs | ||
|
||
**Avoid** | ||
```rego | ||
package policy | ||
import future.keywords.if | ||
deny if { | ||
"admin" != input.user.roles[_] | ||
} | ||
``` | ||
|
||
**Prefer** | ||
```rego | ||
package policy | ||
import future.keywords.if | ||
import future.keywords.in | ||
deny if { | ||
not "admin" in input.user.roles | ||
} | ||
# Or as a one-liner | ||
deny if not "admin" in input.user.roles | ||
``` | ||
|
||
## Rationale | ||
|
||
Likely one of the most common mistakes in Rego is to use `!=` in a loop thinking it means "not in". It took some years | ||
for the `in` keyword to be added to Rego, so perhaps it's not surprising that this mistake is a common one even to this | ||
day. If it doesn't mean "not in", what does it mean? | ||
|
||
```rego | ||
package policy | ||
deny { | ||
"admin" != input.user.roles[_] | ||
} | ||
``` | ||
|
||
The body of the `deny` rule above roughly translates to "for any item in `input.user.roles`, return true if the item is | ||
not `admin`". This is almost never what the policy author intended. What the policy author likely intended was | ||
"deny if `admin` is not in `input.user.roles`". The above policy would thus **not** deny a user with the roles `["user", "admin"]` since the first item in the array is not "admin". This is almost | ||
never what the policy author intended. | ||
|
||
**Note**: This linter rule currently only checks for `!=` in a non-nested comparison where iteration happens on either | ||
side of the comparison in the same expression. This will be improved in time. | ||
|
||
## Configuration Options | ||
|
||
This linter rule provides the following configuration options: | ||
|
||
```yaml | ||
rules: | ||
bugs: | ||
not-equals-in-loop: | ||
# one of "error", "warning", "ignore" | ||
level: error | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package p | ||
|
||
deny { | ||
"admin" != input.user.roles[_] | ||
} |