Skip to content

Test code review #1

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 3 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
19 changes: 19 additions & 0 deletions examples/bad_example.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
def run():
code = "print('Hello World')"
eval(code)

Comment on lines +2 to +4
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

eval() is unnecessary and unsafe

Executing arbitrary strings invites RCE. Replace with a direct call:

-  code = "print('Hello World')"
-  eval(code)
+  print("Hello World")
📝 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
code = "print('Hello World')"
eval(code)
print("Hello World")
🤖 Prompt for AI Agents
In examples/bad_example.py around lines 2 to 4, the use of eval() to execute the
code string is unnecessary and unsafe as it can lead to remote code execution
vulnerabilities. Replace the eval(code) call with a direct execution of the
code, such as using a print statement directly or another safe method that does
not involve evaluating a string.

user_input = input("Enter a number: ")
try:
result = int(user_input) / 0
print( result )
except:
print("something went wrong")
Comment on lines +5 to +10
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guaranteed ZeroDivisionError + bare except

Dividing by zero always throws, and the bare except hides real issues.

-  user_input = input("Enter a number: ")
-  try:
-    result = int(user_input) / 0
-    print(   result  )
-  except:
-    print("something went wrong")
+  user_input = input("Enter a number: ")
+  try:
+    value = float(user_input)
+    result = value / 2          # example divisor
+    print(f"Result: {result}")
+  except ValueError:
+    print("Please enter a valid number.")
📝 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
user_input = input("Enter a number: ")
try:
result = int(user_input) / 0
print( result )
except:
print("something went wrong")
user_input = input("Enter a number: ")
try:
value = float(user_input)
result = value / 2 # example divisor
print(f"Result: {result}")
except ValueError:
print("Please enter a valid number.")
🧰 Tools
🪛 Ruff (0.11.9)

9-9: Do not use bare except

(E722)

🤖 Prompt for AI Agents
In examples/bad_example.py around lines 5 to 10, the code divides by zero which
always raises a ZeroDivisionError, and it uses a bare except that hides other
potential errors. Fix this by removing the division by zero or replacing it with
a valid divisor, and replace the bare except with a specific exception handler
such as ZeroDivisionError or ValueError to properly handle expected errors
without masking others.


temp = []
for i in range(0,10):
temp.append(i)
for i in range(0,10):
temp.append(i)



Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,21 @@ jobs:
if [ -f original_files_temp.json ]; then
jq -s '.[0] * .[1]' diff.json original_files_temp.json > combined.json
mv combined.json diff.json
fi

- name: Display Processed Diff (Debug)
run: cat diff.json

- name: Analyze with OpenAI
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
MODELNAME: ${{ vars.MODELNAME }}
run: |
DIFF_CONTENT=$(jq -r '.diff' diff.json)
ORIGINAL_FILES=$(jq -r '."original files"' diff.json)
PROMPT="Please review the following code changes for any obvious quality or security issues. Provide a brief report in markdown format:\n\nDIFF:\n${DIFF_CONTENT}\n\nORIGINAL FILES:\n${ORIGINAL_FILES}"
jq -n --arg prompt "$PROMPT" '{
"model": "gpt-4",
jq -n --arg model "$MODELNAME" --arg prompt "$PROMPT" '{
"model": "\($model)",
"messages": [
Comment on lines +114 to 116
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Variable already interpolated – lose the extra quotes

jq -n --arg model "$MODELNAME" makes $model a JSON string. Wrapping with "$(…)" re-quotes it:

"model": "\($model)"

results in "model": "\"gpt-4o\"" (note the escaped quotes). Use the raw value instead:

-  jq -n --arg model "$MODELNAME" --arg prompt "$PROMPT" '{
-    "model": "\($model)",
+  jq -n --arg model "$MODELNAME" --arg prompt "$PROMPT" '{
+    "model": $model,
🤖 Prompt for AI Agents
In examples/third_party/Code_quality_and_security_scan_with_GitHub_Actions.md at
lines 114 to 116, the jq command incorrectly wraps the variable $model in extra
quotes, causing it to be double-quoted in the JSON output. To fix this, remove
the surrounding quotes around \($model) so that the value is inserted as a raw
JSON string without additional escaping.

{ "role": "system", "content": "You are a code reviewer." },
{ "role": "user", "content": $prompt }
Expand Down Expand Up @@ -257,4 +259,4 @@ Commit this workflow to your repository, then open a new PR. The workflow will r

![pr_quality_and_security_check.png](../../images/pr_quality_and_security_check.png)

![workflow_check.png](../../images/workflow_check.png)
![workflow_check.png](../../images/workflow_check.png)