Potential fix for code scanning alert no. 10: Clear-text logging of sensitive information - #38
Merged
Conversation
…ensitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a security vulnerability (code scanning alert #10) by removing personally identifiable information (PII) from log messages. The changes prevent contact names and normalized queries from being logged in clear text while maintaining the same functionality.
Changes:
- Removed contact name from success messages in WhatsApp operations (calling, messaging, video calling)
- Removed normalized query (contact name) from contact search success message
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
7 tasks
Contributor
[WIP] Fix clear-text logging of sensitive information
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Potential fix for https://github.com/vannu07/jarvis/security/code-scanning/10
General approach: avoid logging sensitive user data (phone numbers, contact names, free-form messages) and, where logging is desirable for UX, restrict it to non-identifying or sanitized summaries. Since
StatusIndicatoris used widely, the least invasive change is to adjust the specific messages that include contact-identifying information (jarvis_messageand similar) so that they no longer contain names or other PII, while preserving user-facing voice feedback viaspeakas needed.Best concrete fix without changing overall functionality:
backend/feature.py:findContact, keep using the contact’s phone number and name internally, but change the success feedback to something generic that doesn’t include the normalized query (which may be the person’s name). For example, replaceStatusIndicator.success(f"Contact found: {query}")withStatusIndicator.success("Contact found"). This still informs the user that a contact was found without logging PII.whatsApp, stop includingnamein the status message passed toStatusIndicator.success. Instead of:"Message sent successfully to " + name"Calling " + name"Starting video call with " + nameuse generic variants like:
"Message sent successfully.""Calling contact...""Starting video call...".jarvis_message(with the name) forspeakif you consider spoken output not to be “logging”. However, sincespeakalso pushes text toeel.DisplayMessage/eel.receiverText, it’s safer to align that as well with the generic message. I will keepspeak(jarvis_message)as-is only in structure—jarvis_messageitself will become generic and no longer contain the name.These changes only touch the content of strings passed to
StatusIndicatorandspeak; they do not affect external behavior other than making visible messages more privacy-preserving. No new functions or imports are required.Specific locations:
backend/feature.py:StatusIndicator.success(f"Contact found: {query}").jarvis_messageassignments to remove inclusion ofname.No changes are needed in
backend/feedback.pyitself, since its sanitizer and printing mechanism are already generic; the problem is the content being sent into it.Suggested fixes powered by Copilot Autofix. Review carefully before merging.