Skip to content

refactor(git): Enhance PR creation and commit handling #2

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

Merged
merged 4 commits into from
Aug 8, 2024
Merged

refactor(git): Enhance PR creation and commit handling #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be374b9
refactor(app): replace Ollama with Bedrock for PR generation
j2nullify Aug 8, 2024
00c0b3b
refactor(app): improve commit and PR creation processes
j2nullify Aug 8, 2024
af97718
fix(app/make_pull_request.py): use 'gh pr edit' instead of 'gh pr cre…
j2nullify Aug 8, 2024
d1437d0
refactor(make_pull_request): improve PR title generation prompt and c…
j2nullify Aug 8, 2024
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
6 changes: 4 additions & 2 deletions app/commit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ def main():
if len(bash_commands) == 0:
bash_commands = [commit_message]
value = Prompt.ask("Commit message", default=bash_commands[0])
if value.strip() == "":
value = bash_commands[0].strip()
result = subprocess.run(["git", "commit", "-m", value])
if result.returncode == 0:
print("✅ Committed successfully!")
return result

else:
print("❌ Failed to commit. Please try again.")

if __name__ == "__main__":
main()
55 changes: 43 additions & 12 deletions app/make_pull_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,42 @@
import subprocess
from query_ollama import query_ollama
from extract_bash import extract_bash_commands_no_line_split
from app.query_bedrock import query_bedrock
from app.extract_bash import extract_bash_commands_no_line_split

def make_pull_request():
def get_pr_diff():
try:
diff = subprocess.check_output(["git", "diff", "origin/main...HEAD"]).decode()
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability, 'Improper Neutralization of Special Elements used in an OS Command', often referred to as 'OS command injection', occurs when an application fails to properly sanitize input that is passed to system commands. In the given code snippet, the application is executing a Git command using user-supplied input without validating or encoding it to ensure it is safe. This makes it possible for an attacker to inject malicious commands.

How this could be exploited:
An attacker could exploit this vulnerability by manipulating the input to execute unintended system commands. For instance, if an attacker can influence the git branches or parameters being input into the 'git diff' command, they might append a semicolon to the end of a branch name followed by any malicious command they wish to run. This could allow the attacker to execute potentially harmful commands on the system.

Read more:
https://cwe.mitre.org/data/definitions/78.html

⚡ Here's how you might fix this potential vulnerability

The modified code adds exception handling for the subprocess calls. If a subprocess call fails, the CalledProcessError exception is caught and an error message is printed. The function then returns None to indicate that an error occurred. This prevents the program from crashing if a subprocess call fails. The subprocess.run() calls now include the check=True argument, which causes an exception to be raised if the command returns a non-zero exit status.

Please note that AI auto-fixes are currently experimental

Add exception handling for subprocess calls

Suggested change
diff = subprocess.check_output(["git", "diff", "origin/main...HEAD"]).decode()
try:
diff = subprocess.check_output(["git", "diff", "origin/main...HEAD"]).decode()
except subprocess.CalledProcessError as e:
print(f"Error: {str(e)}")
return None

Add exception handling for subprocess calls
Line 16

        try:
            current_branch = subprocess.check_output(["git", "rev-parse", "--abbrev-ref", "HEAD"]).decode().strip()
        except subprocess.CalledProcessError as e:
            print(f"Error: {str(e)}")
            return None

Add exception handling for subprocess calls
Line 19

        try:
            subprocess.run(["git", "push", "--set-upstream", "origin", current_branch], check=True)
        except subprocess.CalledProcessError as e:
            print(f"Error: {str(e)}")
            return None

Add exception handling for subprocess calls
Line 49

        try:
            result = subprocess.run(["gh", "pr", "create", "--title", pr_title, "--body", pr_description], check=True)
        except subprocess.CalledProcessError as e:
            print(f"Error: {str(e)}")
            return None

Add exception handling for subprocess calls
Line 51

        try:
            result = subprocess.run(["gh", "pr", "edit", "--title", pr_title, "--body", pr_description], check=True)
        except subprocess.CalledProcessError as e:
            print(f"Error: {str(e)}")
            return None

Powered by nullify.ai

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

return diff
except subprocess.CalledProcessError:
print("Error: Unable to get the diff. Make sure you're in a git repository.")
return None

def main():
# Push the branch to remote
subprocess.run(["git", "push"])
# Get the current branch name
current_branch = subprocess.check_output(["git", "rev-parse", "--abbrev-ref", "HEAD"]).decode().strip()
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability, 'Improper Neutralization of Special Elements used in an OS Command', occurs when user input or variable data isn't properly sanitized before being used in a system command. This can lead to executing unintended commands or passing invalid data to the system. In the provided code snippet, the output of a Git command is being fetched without ensuring that it doesn't contain malicious modifications. Although the current usage context in the snippet itself might seem benign, using unsanitized outputs further in the application can introduce serious security risks.

How this could be exploited:
An attacker could exploit this vulnerability by injecting special characters or command sequences into the Git repository's branch names. When the application fetches the current branch name, these injections could result in the execution of unintended commands on the system where the script runs. For example, creating a branch named 'master; rm -rf /' could potentially lead to the execution of a harmful command along with the intended Git command.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

This code vulnerability, known as 'Improper Neutralization of Special Elements used in an OS Command', occurs when input data is not properly sanitized before being used in operating system commands. In this snippet, the code is using the Python 'subprocess' module to execute a 'git' command, which retrieves the current Git branch. However, if the output of the command includes special characters or unintended commands, it could potentially lead to the execution of arbitrary commands on the operating system.

How this could be exploited:
An attacker could exploit this by manipulating the Git branch name to include special OS command elements. For example, if an attacker creates a branch named 'master; rm -rf /', and the code does not properly neutralize the special characters, the 'rm -rf /' command could be executed unintentionally, leading to deletion of files.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)


# Set the upstream to origin and push the current branch
subprocess.run(["git", "push", "--set-upstream", "origin", current_branch])
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability 'Improper Neutralization of Special Elements used in an OS Command' occurs when an application does not properly sanitize input that is passed to the operating system. This can lead to command injection, where an attacker can control the command that is executed on the operating system. In the provided code snippet, the variable 'current_branch' is being used to dynamically create the command that is passed to the 'subprocess.run' method. If 'current_branch' comes from an unreliable source, or if it's manipulable by an external user without proper validation and sanitization, it could lead to a security risk.

How this could be exploited:
An attacker could exploit this vulnerability by injecting special characters or command operators into the 'current_branch' variable. For example, setting 'current_branch' to a value like '; rm -rf /' could result in the deletion of the server's root directory if the script is executed with sufficient permissions, leading to catastrophic data loss and system failure.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability 'Improper Neutralization of Special Elements used in an OS Command' arises when input data isn't correctly sanitized before being passed to an operating system command. In simpler terms, if the data that a user or another part of the application provides contains special characters or command operators, these might be interpreted by the operating system as instructions, rather than plain data. In the provided code snippet, the variable 'current_branch' is included directly in a system command that utilizes the 'subprocess.run' function to execute a 'git push' command. If 'current_branch' contains any special characters or unintended command sequences, these will be executed by the shell, leading to potential malicious operations.

How this could be exploited:
An attacker could exploit this vulnerability by manipulating the 'current_branch' variable to include additional shell commands. For example, if an attacker manages to set 'current_branch' to '; rm -rf /', this would translate the command to 'git push --set-upstream origin; rm -rf /', which would execute a destructive 'remove' command on the root directory of the server, likely causing significant damage or data loss.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)


pr_diff = get_pr_diff()
if not pr_diff:
return

# Generate PR description using Ollama
# Generate PR title and description using Ollama
prompt_title = "Generate a concise and informative pull request title for a feature branch."
pr_title = query_ollama(prompt_title)
# Generate PR title and description using Bedrock
prompt_title = f"""Generate a concise and informative pull request title based on the following diff:

{pr_diff}

Respond with only the title, enclosed in triple backticks (```). For example:
```
feat(user-auth): Implement JWT-based authentication
```
"""
pr_title = query_bedrock(prompt_title)
pr_title = extract_bash_commands_no_line_split(pr_title)[0]

prompt_body = "Generate a concise and informative pull request description for a feature branch. Include key changes and their impact."
pr_description = query_ollama(prompt_body)
prompt_body = f"Generate a concise and informative pull request description based on the following diff. Include key changes and their impact:\n\n{pr_diff}"
pr_description = query_bedrock(prompt_body)
pr_description = extract_bash_commands_no_line_split(pr_description)[0]

# Create pull request using GitHub CLI
Expand All @@ -30,7 +53,15 @@ def make_pull_request():
return

# If gh is installed, proceed with creating the pull request
subprocess.run(["gh", "pr", "create", "--title", pr_title, "--body", pr_description])

result = subprocess.run(["gh", "pr", "create", "--title", pr_title, "--body", pr_description])
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability identified is 'Improper Neutralization of Special Elements used in an OS Command,' often referred to as command injection. This occurs when input data containing special characters or commands is fed directly into system-level command execution without proper sanitization. In the provided code example, variables 'pr_title' and 'pr_description' are included directly in a command that creates a pull request via the GitHub CLI ('gh'). If these variables are taken from user inputs and not properly sanitized, they may include malicious commands that could be executed on the operating system.

How this could be exploited:
An attacker could exploit this by crafting a pull request title or description that includes OS command injections. For example, if an attacker uses a pr_title like 'Update; rm -rf /', it could lead to the deletion of files or directories on the server (depending on permissions), when the script is executed.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

if result.returncode != 0:
result = subprocess.run(["gh", "pr", "edit", "--title", pr_title, "--body", pr_description])
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The vulnerability identified, known as 'Improper Neutralization of Special Elements used in an OS Command' or 'Command Injection', occurs when user-supplied input (variables like pr_title or pr_description) is not properly sanitized before being included in operating system commands. Here, the code directly incorporates inputs pr_title and pr_description into the OS command to edit a pull request. If these inputs aren't properly validated or sanitized, they could be crafted to execute unintended additional commands alongside the intended ones.

How this could be exploited:
An attacker could exploit this vulnerability by embedding special command-separated characters in the pr_title or pr_description inputs. For example, setting pr_description to 'Valid description here && delete_all_files' would lead to the deletion of all files (implicitly executed by the command) if the code does not sanitize this input. This, in turn, triggers the additional dangerous command hidden within the legitimate command.

Read more:
https://cwe.mitre.org/data/definitions/78.html

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)

Comment on lines +57 to +59
Copy link

Choose a reason for hiding this comment

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

Nullify Code Language: Python 🔵 MEDIUM Severity CWE-78

Improper Neutralization of Special Elements used in an OS Command

The identified vulnerability 'Improper Neutralization of Special Elements used in an OS Command' occurs when input data isn't properly sanitized before being used in an operating system command. This can allow attackers to inject unintended commands to be executed by the system. In the provided code, 'pr_title' and 'pr_description' are included as parameters to a command line operation without ensuring they are safe to employ. This implies that if the pull request title or description contains malicious command sequences, they might be executed.

How this could be exploited:
An example exploitation could occur if an attacker inputs specially crafted text into either 'pr_title' or 'pr_description' that includes shell control characters or commands. For instance, if the title is crafted like 'Valid Title; rm -rf /', this would not only set the title but also attempt to delete all files on the server running this script, if it has sufficient permissions.

Read more:
https://cwe.mitre.org/data/definitions/78.html

⚡ Here's how you might fix this potential vulnerability

The modified code mitigates this vulnerability by escaping any special characters in the pr_title and pr_description using the shlex.quote function. This ensures that these inputs are treated as literal strings by the shell and not interpreted as part of the command.

Please note that AI auto-fixes are currently experimental

Escape special characters in pr_title and pr_description

Suggested change
result = subprocess.run(["gh", "pr", "create", "--title", pr_title, "--body", pr_description])
if result.returncode != 0:
result = subprocess.run(["gh", "pr", "edit", "--title", pr_title, "--body", pr_description])
import shlex
pr_title = shlex.quote(pr_title)
pr_description = shlex.quote(pr_description)
result = subprocess.run(["gh", "pr", "create", "--title", pr_title, "--body", pr_description])
if result.returncode != 0:
result = subprocess.run(["gh", "pr", "edit", "--title", pr_title, "--body", pr_description])

Powered by nullify.ai

Reply with /nullify to interact with me like another developer
(you will need to refresh the page for updates)


if result.returncode == 0:
print("✅ Pull request created successfully!")
else:
print("❌ Failed to create pull request. Please try again.")

if __name__ == "__main__":
make_pull_request()
main()
1 change: 1 addition & 0 deletions app/query_bedrock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def query_bedrock(prompt: str, stream: bool = True):
if "delta" in chunk:
if "text" in chunk["delta"]:
text = chunk["delta"]["text"]
print(text, end="")
full_response += text
print() # Print a newline at the end
else:
Expand Down