-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix code scanning alert #78: Log Injection #53
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback from Senior Dev Bot
API/route.py
Outdated
|
||
""" | ||
logging.info('Deleting Employee') | ||
logging.debug(f"Deleting for EmployeeCode: {EmployeeCode}") | ||
sanitized_employee_code = re.sub(r'\D', '', str(EmployeeCode)) | ||
logging.debug(f"Deleting for EmployeeCode: {sanitized_employee_code}") | ||
client2.find_one_and_delete(collection2, {'EmployeeCode': EmployeeCode}) | ||
|
||
return {'Message': 'Successfully Deleted'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CODE REVIEW
Good job on sanitizing the EmployeeCode
before logging! However, ensure that the original variable remains unchanged for further operations. Alternatively, consider logging the sanitized value conditionally based on the logging level to avoid unnecessary processing.
sanitized_employee_code = re.sub(r'\D', '', str(EmployeeCode))
if logging.getLogger().isEnabledFor(logging.DEBUG):
logging.debug(f"Deleting for EmployeeCode: {sanitized_employee_code}")
Quality Gate passedIssues Measures |
for more information, see https://pre-commit.ci
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
Fixes https://github.com/Devasy23/FaceRec/security/code-scanning/78
Suggested fixes powered by Copilot Autofix. Review carefully before merging.