-
Notifications
You must be signed in to change notification settings - Fork 401
Chatbot impl #295
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
base: develop
Are you sure you want to change the base?
Chatbot impl #295
Conversation
* added llm chatbot service
* removed unused imports
except Exception as e: | ||
app.logger.error("Error initializing bot ", e) | ||
app.logger.debug("Error initializing bot ", e, exc_info=True) | ||
return jsonify({"message": "Not able to initialize model " + str(e)}), 500 |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
To fix the issue, we need to ensure that exception details are not exposed to the user. Instead, we should log the full exception details on the server for debugging purposes and return a generic error message to the user. This approach aligns with best practices for handling exceptions securely.
- Replace the response message on line 173 with a generic error message, such as
"An internal error occurred. Please try again later."
. - Log the full exception details, including the stack trace, using
app.logger.debug
orapp.logger.error
withexc_info=True
. This ensures that developers can still access the details for debugging without exposing them to the user.
-
Copy modified lines R171-R172
@@ -170,5 +170,4 @@ | ||
except Exception as e: | ||
app.logger.error("Error initializing bot %s", e) | ||
app.logger.debug("Error initializing bot %s", e, exc_info=True) | ||
return jsonify({"message": "Not able to initialize model " + str(e)}), 500 | ||
app.logger.error("Error initializing bot", exc_info=True) | ||
return jsonify({"message": "An internal error occurred. Please try again later."}), 500 | ||
|
) | ||
except Exception as e: | ||
app.logger.error("Error checking state ", e) | ||
return jsonify({"message": "Error checking state " + str(e)}, 200) |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
To fix the issue, we will replace the detailed error message sent to the user with a generic error message. The detailed exception information will instead be logged on the server for debugging purposes. This ensures that sensitive information is not exposed to external users while still allowing developers to diagnose issues using server logs.
-
Copy modified lines R193-R194
@@ -192,4 +192,4 @@ | ||
except Exception as e: | ||
app.logger.error("Error checking state ", e) | ||
return jsonify({"message": "Error checking state " + str(e)}, 200) | ||
app.logger.error("Error checking state: %s", e, exc_info=True) | ||
return jsonify({"message": "An internal error occurred while checking state."}), 500 | ||
return ( |
|
||
|
||
if __name__ == "__main__": | ||
app.run(host="0.0.0.0", port=5002, debug=True) |
Check failure
Code scanning / CodeQL
Flask app is run in debug mode High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
To address the issue, we will ensure that the debug=True
parameter is not used in production. This can be achieved by introducing an environment variable (e.g., FLASK_DEBUG
) to control the debug mode. The app.run()
method will then use this variable to determine whether to enable debug mode. This approach allows developers to enable debug mode during development while ensuring it is disabled in production.
Changes to be made:
- Replace the hardcoded
debug=True
with a conditional check that reads theFLASK_DEBUG
environment variable. - Update the
app.run()
call to use the value of this variable. - Add a default value (
False
) for the debug mode if the environment variable is not set.
-
Copy modified lines R249-R250
@@ -248,3 +248,4 @@ | ||
if __name__ == "__main__": | ||
app.run(host="0.0.0.0", port=5002, debug=True) | ||
debug_mode = os.getenv("FLASK_DEBUG", "False").lower() in ("true", "1", "yes") | ||
app.run(host="0.0.0.0", port=5002, debug=debug_mode) | ||
else: |
Test Results92 tests 92 ✅ 2s ⏱️ Results for commit 3a9b002. ♻️ This comment has been updated with latest results. |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
app = Flask(__name__) | ||
app.logger.setLevel(logging.DEBUG) | ||
|
||
app.logger.debug("MONGO_CONNECTION_URI:: %s", MONGO_CONNECTION_URI) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
sensitive data (password)
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 12 hours ago
To fix the issue, we should avoid logging sensitive information such as MONGO_CONNECTION_URI
. Instead, we can log a sanitized or redacted version of the URI that excludes sensitive details like the password. This ensures that the logs remain useful for debugging purposes without exposing sensitive data.
The fix involves:
- Redacting the password from
MONGO_CONNECTION_URI
before logging it. - Updating the log statement on line 20 in
services/chatbot/src/chatbot_api.py
to use the sanitized version.
-
Copy modified lines R20-R21
@@ -19,3 +19,4 @@ | ||
|
||
app.logger.debug("MONGO_CONNECTION_URI:: %s", MONGO_CONNECTION_URI) | ||
redacted_uri = MONGO_CONNECTION_URI.replace(MONGO_PASSWORD, "REDACTED") | ||
app.logger.debug("MONGO_CONNECTION_URI:: %s", redacted_uri) | ||
retriever = None |
-
Copy modified line R4
@@ -3,3 +3,3 @@ | ||
MONGO_USER = os.environ.get("MONGO_USER", "admin") | ||
MONGO_PASSWORD = os.environ.get("MONGO_PASSWORD", "crapisecretpassword") | ||
MONGO_PASSWORD = os.environ.get("MONGO_PASSWORD", "crapisecretpassword") # Used for database connection and redaction | ||
MONGO_HOST = os.environ.get("MONGO_HOST", "mongodb") |
Description
Implement a vulnerable chatbot
Testing
Local testing
Documentation
Make sure that you have documented corresponding changes in this repository.
Checklist: