Skip to content

Regal showcase #30

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .coderabbit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
reviews:
path_filters: ["**/*.yml","**/*.yaml","**/*.rego"]
41 changes: 41 additions & 0 deletions broken.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package authz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add import rego.v1 for modern Rego compatibility.

Modern Rego policies should include the v1 import for better compatibility and to enable v1 features.

Add this after the package declaration:

 package authz
+
+import rego.v1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
package authz
package authz
import rego.v1
🧰 Tools
🪛 Regal (0.35.1)

[error] 1-1: Directory structure should mirror package

(idiomatic)


[error] 1-1: Use import rego.v1

(imports)

🤖 Prompt for AI Agents
In broken.rego at line 1, the policy is missing the import statement for
rego.v1, which is required for modern Rego compatibility and to enable v1
features. Add the line 'import rego.v1' immediately after the package
declaration to fix this.


default allow = false

ALLOW := true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused constant ALLOW.

This constant is defined but never referenced in the policy.

🤖 Prompt for AI Agents
In broken.rego at line 5, the constant ALLOW is defined but never used in the
policy. Remove the line defining ALLOW to clean up unused code and avoid
confusion.


x := 1
x := 2
Comment on lines +7 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix duplicate assignment to variable x.

Variable x is assigned two different values, which will cause a compilation error in Rego.

Remove one of the assignments or use different variable names:

-x := 1
-x := 2
+x1 := 1
+x2 := 2
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
x := 1
x := 2
x1 := 1
x2 := 2
🤖 Prompt for AI Agents
In broken.rego at lines 7 to 8, the variable x is assigned twice, which causes a
compilation error. Fix this by removing one of the assignments or renaming one
of the variables to avoid duplicate assignment.


unused1 := "test"
unused2 := input.fake.value
Comment on lines +10 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused variables.

Variables unused1 and unused2 are declared but never used, violating the no-unused-vars rule.

🤖 Prompt for AI Agents
In broken.rego around lines 10 to 11, remove the declarations of the unused
variables `unused1` and `unused2` since they are not used anywhere in the code
and violate the no-unused-vars rule.


deny[msg] {
some x
input.user == "admin"
input.user == "admin"
msg := "Access granted"
}
Comment on lines +13 to +18
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix contradictory logic in deny rule.

The deny rule returns "Access granted" which is semantically incorrect. Also, it has redundant conditions and an unused variable declaration.

Apply this fix:

 deny[msg] {
-  some x
-  input.user == "admin"
   input.user == "admin"
-  msg := "Access granted"
+  msg := "Access denied: admin access restricted"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
deny[msg] {
some x
input.user == "admin"
input.user == "admin"
msg := "Access granted"
}
deny[msg] {
input.user == "admin"
msg := "Access denied: admin access restricted"
}
🧰 Tools
🪛 Regal (0.35.1)

[error] 15-15: Non-loop expression

(performance)


[error] 16-16: Non-loop expression

(performance)

🤖 Prompt for AI Agents
In broken.rego around lines 13 to 18, the deny rule incorrectly returns "Access
granted" and contains redundant conditions checking input.user == "admin" twice,
as well as an unused variable x. Remove the duplicate condition and the unused
variable, and change the message to reflect denial of access appropriately, such
as "Access denied".


allow {
x == "user"
x := "user"
count(x)
true
}
Comment on lines +20 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix logical errors in allow rule.

The rule has several issues:

  1. Checks x == "user" before assigning it
  2. count(x) on a string returns its length but the result is unused
  3. Constant true condition is redundant

Consider this corrected logic:

 allow {
-  x == "user"
-  x := "user"
-  count(x)
-  true
+  input.user == "user"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
allow {
x == "user"
x := "user"
count(x)
true
}
allow {
input.user == "user"
}
🧰 Tools
🪛 Regal (0.35.1)

[error] 24-24: Constant condition

(bugs)


[error] 23-23: Non-boolean return value unassigned

(bugs)

🤖 Prompt for AI Agents
In broken.rego around lines 20 to 25, the allow rule has logical errors: it
compares x to "user" before assigning x, uses count(x) on a string without using
the result, and includes a redundant true condition. Fix this by first assigning
x to "user", removing the count(x) call since its result is unused, and
eliminating the constant true condition to simplify the rule logic.


result := {
"status": 200,
"message": msg
}
Comment on lines +27 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix undefined variable msg in result object.

The msg variable is not defined in this scope.

Either define msg or remove this object if it's not needed:

 result := {
   "status": 200,
-  "message": msg
+  "message": "OK"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
result := {
"status": 200,
"message": msg
}
result := {
"status": 200,
"message": "OK"
}
🤖 Prompt for AI Agents
In broken.rego around lines 27 to 30, the variable `msg` used in the result
object is undefined in this scope. To fix this, either define the `msg` variable
with an appropriate value before using it in the result object or remove the
`message` field from the result object if it is not necessary.


foo {
bar := [1, 2, 3]
bar = bar
}
Comment on lines +32 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove or fix meaningless foo rule.

The rule assigns a value and then checks if it equals itself, which is always true and serves no purpose.

Either remove this rule or implement meaningful logic.

🤖 Prompt for AI Agents
In broken.rego around lines 32 to 35, the foo rule assigns a list to bar and
then compares bar to itself, which is always true and redundant. Remove this
rule entirely if it is not needed, or replace it with meaningful logic that
performs a useful check or computation relevant to your policy.


token_check {
x := split("abc.def.ghi", ".")
x[1] == "def"
}
Comment on lines +37 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using input data instead of hardcoded string.

The token check uses a hardcoded string which isn't practical for real token validation.

Consider using input data:

 token_check {
-  x := split("abc.def.ghi", ".")
+  x := split(input.token, ".")
   x[1] == "def"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
token_check {
x := split("abc.def.ghi", ".")
x[1] == "def"
}
token_check {
x := split(input.token, ".")
x[1] == "def"
}
🤖 Prompt for AI Agents
In broken.rego around lines 37 to 40, the token_check rule uses a hardcoded
string "abc.def.ghi" for splitting and checking, which is not practical for real
token validation. Replace the hardcoded string with a dynamic value from the
input data, such as input.token, to make the rule validate tokens based on
actual input rather than a fixed string.


16 changes: 16 additions & 0 deletions regal_config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
format:
max-line-length: 100
indentation: 2

lint:
enabled: true
severity:
default: error # Escalated for CI to catch violations

rules:
- no-unused-vars
- no-shadowing
- test-missing-assertion
- test-does-nothing
- var-uppercase