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.

Sign up for GitHub

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

Jump to bottom

Add REGO debugger to Mindev. #5229

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add REGO debugger to Mindev. #5229

wants to merge 2 commits into from

Conversation

blkt
Copy link
Contributor

@blkt blkt commented Dec 19, 2024

Summary

This change adds the possibility to start evaluate a REGO-based rule type in a debugger.

The debugger allows setting breakpoints, stepping, printing source, and a few other simple utilities.

The debugger is currently very, very, VERY rough around the edges and could use some love, especially in the reception of events from the debuggee, which is done inline and not asynchronously.

Change Type

  • Bug fix (resolves an issue without affecting existing features)
  • Feature (adds new functionality without breaking changes)
  • Breaking change (may impact existing functionalities or require documentation updates)
  • Documentation (updates or additions to documentation)
  • Refactoring or test improvements (no bug fixes or new functionality)

Testing

Mostly untested, hic sunt dracones.

Review Checklist:

  • Reviewed my own code for quality and clarity.
  • Added comments to complex or tricky code sections.
  • Updated any affected documentation.
  • Included tests that validate the fix or feature.
  • Checked that related changes are merged.

@blkt blkt added the hackathon label Dec 19, 2024
@blkt blkt self-assigned this Dec 19, 2024
@blkt blkt requested a review from a team as a code owner December 19, 2024 22:39
@blkt blkt marked this pull request as draft December 19, 2024 22:44
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 19, 2024
View reviewed changes
internal/engine/eval/rego/debug.go Fixed Show fixed Hide fixed
internal/engine/eval/rego/debug.go
if err != nil {
return debug.BreakpointID(-1), fmt.Errorf(`%w: invalid breakpoint id %s`, errInvalidBP, num)
}
return debug.BreakpointID(i), nil

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
Loading
to a lower bit size type int without an upper bound check.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the parsed integer is within the valid range for debug.BreakpointID before performing the conversion. This can be done by adding an upper bound check after parsing the integer. If the parsed integer exceeds the maximum value for debug.BreakpointID, we should return an error.

  1. Add an upper bound check after parsing the integer.
  2. Ensure that the parsed integer is within the valid range for debug.BreakpointID.
  3. Return an error if the parsed integer is out of bounds.
Suggested changeset 1
internal/engine/eval/rego/debug.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/engine/eval/rego/debug.go b/internal/engine/eval/rego/debug.go
--- a/internal/engine/eval/rego/debug.go
+++ b/internal/engine/eval/rego/debug.go
@@ -565,5 +565,5 @@
 
-	if i < 1 {
+	if i < 1 || i > math.MaxInt32 {
 		return debug.BreakpointID(-1), fmt.Errorf(
-			"%w: negative line id",
+			"%w: invalid line id",
 			errInvalidBP,
EOF
@@ -565,5 +565,5 @@

if i < 1 {
if i < 1 || i > math.MaxInt32 {
return debug.BreakpointID(-1), fmt.Errorf(
"%w: negative line id",
"%w: invalid line id",
errInvalidBP,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@blkt blkt force-pushed the feat/rego-debugger-awesomeness branch 6 times, most recently from 03a9952 to 2fd436e Compare December 20, 2024 22:34
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 20, 2024
View reviewed changes
internal/engine/eval/rego/debug.go
if err != nil {
return nil, fmt.Errorf(`%w: invalid line "%s": %s`, errInvalidBP, num, err)
}
if i < 1 || int(i) > lineCount {

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
Loading
to a lower bit size type int without an upper bound check.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the value parsed by strconv.ParseInt fits within the range of the int type before converting it. This can be done by adding an upper bound check against math.MaxInt (the maximum value for the int type). If the value exceeds this limit, we should return an error.

Suggested changeset 1
internal/engine/eval/rego/debug.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/engine/eval/rego/debug.go b/internal/engine/eval/rego/debug.go
--- a/internal/engine/eval/rego/debug.go
+++ b/internal/engine/eval/rego/debug.go
@@ -536,3 +536,3 @@
 	}
-	if i < 1 || int(i) > lineCount {
+	if i < 1 || i > math.MaxInt || int(i) > lineCount {
 		return nil, fmt.Errorf("%w: invalid line %d", errInvalidBP, i)
@@ -565,5 +565,5 @@
 
-	if i < 1 {
+	if i < 1 || i > math.MaxInt {
 		return debug.BreakpointID(-1), fmt.Errorf(
-			"%w: negative line id",
+			"%w: invalid line id",
 			errInvalidBP,
EOF
@@ -536,3 +536,3 @@
}
if i < 1 || int(i) > lineCount {
if i < 1 || i > math.MaxInt || int(i) > lineCount {
return nil, fmt.Errorf("%w: invalid line %d", errInvalidBP, i)
@@ -565,5 +565,5 @@

if i < 1 {
if i < 1 || i > math.MaxInt {
return debug.BreakpointID(-1), fmt.Errorf(
"%w: negative line id",
"%w: invalid line id",
errInvalidBP,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
internal/engine/eval/rego/debug.go
if i < 1 || int(i) > lineCount {
return nil, fmt.Errorf("%w: invalid line %d", errInvalidBP, i)
}
return &location.Location{File: "minder.rego", Row: int(i)}, nil

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
Loading
to a lower bit size type int without an upper bound check.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the parsed integer value fits within the bounds of an int before performing the conversion. This can be done by adding an upper bound check against the maximum value of an int type. We will use the math.MaxInt constant to perform this check.

Suggested changeset 1
internal/engine/eval/rego/debug.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/engine/eval/rego/debug.go b/internal/engine/eval/rego/debug.go
--- a/internal/engine/eval/rego/debug.go
+++ b/internal/engine/eval/rego/debug.go
@@ -536,3 +536,3 @@
 	}
-	if i < 1 || int(i) > lineCount {
+	if i < 1 || i > int64(lineCount) || i > math.MaxInt {
 		return nil, fmt.Errorf("%w: invalid line %d", errInvalidBP, i)
EOF
@@ -536,3 +536,3 @@
}
if i < 1 || int(i) > lineCount {
if i < 1 || i > int64(lineCount) || i > math.MaxInt {
return nil, fmt.Errorf("%w: invalid line %d", errInvalidBP, i)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
internal/engine/eval/rego/debug.go
)
}

if !slices.Contains(ids, debug.BreakpointID(i)) {

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
Loading
to a lower bit size type int without an upper bound check.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the parsed integer value falls within the valid range for the debug.BreakpointID type before performing the conversion. This can be achieved by adding an upper bound check using the maximum value for the debug.BreakpointID type.

  1. Use strconv.ParseInt with a bit size of 32 to directly parse the string into a 32-bit integer if debug.BreakpointID is a 32-bit type.
  2. Alternatively, add explicit bounds checks to ensure the parsed value is within the valid range for debug.BreakpointID.
Suggested changeset 1
internal/engine/eval/rego/debug.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/engine/eval/rego/debug.go b/internal/engine/eval/rego/debug.go
--- a/internal/engine/eval/rego/debug.go
+++ b/internal/engine/eval/rego/debug.go
@@ -557,3 +557,3 @@
 
-	i, err := strconv.ParseInt(num, 10, 64)
+	i, err := strconv.ParseInt(num, 10, 32)
 	if err != nil {
@@ -565,5 +565,5 @@
 
-	if i < 1 {
+	if i < 1 || i > math.MaxInt32 {
 		return debug.BreakpointID(-1), fmt.Errorf(
-			"%w: negative line id",
+			"%w: invalid line id",
 			errInvalidBP,
EOF
@@ -557,3 +557,3 @@

i, err := strconv.ParseInt(num, 10, 64)
i, err := strconv.ParseInt(num, 10, 32)
if err != nil {
@@ -565,5 +565,5 @@

if i < 1 {
if i < 1 || i > math.MaxInt32 {
return debug.BreakpointID(-1), fmt.Errorf(
"%w: negative line id",
"%w: invalid line id",
errInvalidBP,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@blkt blkt force-pushed the feat/rego-debugger-awesomeness branch 2 times, most recently from ef3e360 to 4edfdb7 Compare December 20, 2024 23:09
@blkt
Add REGO debugger to Mindev.
ddccec9 
This change adds the possibility to start evaluate a REGO-based rule
type in a debugger.

The debugger allows setting breakpoints, stepping, printing source,
and a few other simple utilities.

The debugger is currently very, very, VERY rough around the edges and
could use some love, especially in the reception of events from the
debuggee, which is done inline and not asynchronously.
@blkt blkt force-pushed the feat/rego-debugger-awesomeness branch 2 times, most recently from 5e66be4 to 3515312 Compare December 23, 2024 14:31
@blkt blkt force-pushed the feat/rego-debugger-awesomeness branch from 3515312 to 586947b Compare December 23, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@blkt blkt

Labels
hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@blkt