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

[CM 27] Created CM to Json Parsing Endpoint #23

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

[CM 27] Created CM to Json Parsing Endpoint #23

wants to merge 2 commits into from

Conversation

eldarhasanov079
Copy link

@eldarhasanov079 eldarhasanov079 commented Nov 16, 2024 •
edited
Loading

Jira Ticket

Jira Ticket

Description

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Changes

Testing

Checklist

  • My branch name matches the format: <ticket-id>/<brief-description-of-change>
  • My PR name matches the format: [<ticket-id>] <brief-description-of-change>
  • I have added doc-comments to all new functions (JSDoc for JS and Docstrings for Python)
  • I have reviewed all of my code
  • My code only contains major changes related to my ticket

Screenshots/Video

Additional Notes

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 16, 2024
View reviewed changes
progressReport/app.py
parsed_json, output_path = parser2.to_json(school_name, course_name, term, start_date, class_levels, student_levels, root)
return jsonify({"parsed_data": parsed_json, "file_saved": output_path}), 200
except Exception as e:
return jsonify({"error": str(e)}), 500

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
Loading
flows to this location and may be exposed to an external user.

Copilot Autofix AI 4 months ago

To fix the problem, we need to ensure that detailed error information is not exposed to the user. Instead, we should log the detailed error information on the server and return a generic error message to the user. This can be achieved by modifying the exception handling block to log the exception and return a generic error message.

  • Modify the exception handling block in the parse_text function to log the exception using Python's logging module.
  • Return a generic error message to the user instead of the detailed exception message.
Suggested changeset 1
progressReport/app.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/progressReport/app.py b/progressReport/app.py
--- a/progressReport/app.py
+++ b/progressReport/app.py
@@ -67,3 +67,5 @@
 
+import logging
 app = Flask(__name__)
+app.logger.setLevel(logging.ERROR)
 
@@ -140,3 +142,4 @@
     except Exception as e:
-        return jsonify({"error": str(e)}), 500
+        app.logger.error("Exception occurred", exc_info=True)
+        return jsonify({"error": "An internal error has occurred!"}), 500
     
EOF
@@ -67,3 +67,5 @@

import logging
app = Flask(__name__)
app.logger.setLevel(logging.ERROR)

@@ -140,3 +142,4 @@
except Exception as e:
return jsonify({"error": str(e)}), 500
app.logger.error("Exception occurred", exc_info=True)
return jsonify({"error": "An internal error has occurred!"}), 500

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
progressReport/parser2.py
"fillcolor": "#{}".format(style_match.group(4))
}
elif parse_mode == "CLASS_LEVEL":
level_match = re.search(r"\s*([A-Za-z-_\s]+): #([A-Za-z0-9]+)", line)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on a
user-provided value
Loading
may run slow on strings with many repetitions of '\t'.
progressReport/parser2.py
level_match = re.search(r"\s*([A-Za-z-_\s]+): #([A-Za-z0-9]+)", line)
class_levels.append({"name": level_match.group(1), "color": "#{}".format(level_match.group(2))})
elif parse_mode == "STUDENT_LEVEL":
level_match = re.search(r"\s*([A-Za-z-_\s]+): #([A-Za-z0-9]+)", line)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on a
user-provided value
Loading
may run slow on strings with many repetitions of '\t'.
progressReport/parser2.py
level_match = re.search(r"\s*([A-Za-z-_\s]+): #([A-Za-z0-9]+)", line)
student_levels.append({"name": level_match.group(1), "color": "#{}".format(level_match.group(2))})
elif parse_mode == "NODE":
node_match = re.search(r"(\s+)([A-Za-z0-9\-\s\\/]+) \[([A-Za-z0-9]+), Week([0-9]+)]", line)

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on a
user-provided value
Loading
may run slow on strings with many repetitions of ' '.
This
regular expression
Loading
that depends on a
user-provided value
Loading
may run slow on strings starting with ' ' and with many repetitions of '\t\t'.
progressReport/parser2.py
}

output_path = 'data/{}_{}.json'.format(school_name, course_name)
with open(output_path, 'w', encoding='utf-8') as json_out_file:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.

Copilot Autofix AI 4 months ago

To fix the problem, we need to ensure that the file paths constructed from user input are validated and sanitized to prevent path traversal attacks. We can achieve this by normalizing the paths and ensuring they remain within a designated safe directory.

  1. Normalize the constructed file path using os.path.normpath to remove any ".." segments.
  2. Check that the normalized path starts with the intended base directory.
  3. Use secure_filename to sanitize the file names.
Suggested changeset 2
progressReport/parser2.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/progressReport/parser2.py b/progressReport/parser2.py
--- a/progressReport/parser2.py
+++ b/progressReport/parser2.py
@@ -139,3 +139,6 @@
 
-    output_path = 'data/{}_{}.json'.format(school_name, course_name)
+    base_path = 'data'
+    output_path = os.path.normpath(os.path.join(base_path, '{}_{}.json'.format(school_name, course_name)))
+    if not output_path.startswith(base_path):
+        raise Exception("Invalid file path")
     with open(output_path, 'w', encoding='utf-8') as json_out_file:
EOF
@@ -139,3 +139,6 @@

output_path = 'data/{}_{}.json'.format(school_name, course_name)
base_path = 'data'
output_path = os.path.normpath(os.path.join(base_path, '{}_{}.json'.format(school_name, course_name)))
if not output_path.startswith(base_path):
raise Exception("Invalid file path")
with open(output_path, 'w', encoding='utf-8') as json_out_file:
progressReport/app.py
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/progressReport/app.py b/progressReport/app.py
--- a/progressReport/app.py
+++ b/progressReport/app.py
@@ -133,4 +133,4 @@
     content = request.data.decode('utf-8')
-    school_name = request.args.get("school_name", "Berkeley")
-    course_name = request.args.get("course_name", "CS10")
+    school_name = secure_filename(request.args.get("school_name", "Berkeley"))
+    course_name = secure_filename(request.args.get("course_name", "CS10"))
     try:
@@ -145,4 +145,4 @@
 def parse():
-    school_name = request.args.get("school_name", "Berkeley")
-    course_name = request.form.get("course_name", "CS10")
+    school_name = secure_filename(request.args.get("school_name", "Berkeley"))
+    course_name = secure_filename(request.form.get("course_name", "CS10"))
     parser2.generate_map(school_name=secure_filename(school_name), course_name=secure_filename(course_name), render=False)
EOF
@@ -133,4 +133,4 @@
content = request.data.decode('utf-8')
school_name = request.args.get("school_name", "Berkeley")
course_name = request.args.get("course_name", "CS10")
school_name = secure_filename(request.args.get("school_name", "Berkeley"))
course_name = secure_filename(request.args.get("course_name", "CS10"))
try:
@@ -145,4 +145,4 @@
def parse():
school_name = request.args.get("school_name", "Berkeley")
course_name = request.form.get("course_name", "CS10")
school_name = secure_filename(request.args.get("school_name", "Berkeley"))
course_name = secure_filename(request.form.get("course_name", "CS10"))
parser2.generate_map(school_name=secure_filename(school_name), course_name=secure_filename(course_name), render=False)
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
@Connor-Bernard Connor-Bernard marked this pull request as draft November 26, 2024 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eldarhasanov079 @eldarh079