Skip to content

fix: update group conversation logic to include authenticated user pa…#6

Merged
ChristopherJHart merged 2 commits intomainfrom
fix-non-participant-conversation-leak
Aug 6, 2025
Merged

fix: update group conversation logic to include authenticated user pa…#6
ChristopherJHart merged 2 commits intomainfrom
fix-non-participant-conversation-leak

Conversation

@ChristopherJHart
Copy link
Collaborator

Summary

Fixes a critical bug where conversations were displayed to users even when they were not participants in those conversations. This was caused by incorrect user ID handling in the group conversation filtering logic.

Problem

The application was showing conversations from Webex spaces where the authenticated user was not a participant, creating a leak.

Root Cause

The bug was in group_group_conversations() function in summarizer/grouping.py at line 275:

# BEFORE (buggy code)
user_id = space_messages[0].sender.id if space_messages else None

This incorrectly used the first message sender's ID as the "authenticated user ID" instead of the actual authenticated user's ID, causing the participation filtering logic to fail.

Solution

Core Fix

  1. Updated function signature: Added user_id parameter to group_group_conversations()
  2. Fixed participation filtering: Replaced incorrect user_id assignment with proper participation checking:
    # AFTER (fixed code)
    user_participated = any(m.sender.id == user_id for m in space_messages)
    if not user_participated:
        continue
  3. Updated function call: Modified group_all_conversations() to pass the authenticated user's ID

Additional Improvements

  • Enhanced test coverage: Added test_non_participant_conversation_leak_bug() that specifically reproduces and validates the fix for this privacy issue
  • Updated existing tests: Modified existing tests to work with the corrected behavior and new function signature

Testing

  • New test: test_non_participant_conversation_leak_bug specifically validates the fix
  • Regression tests: All existing tests updated and passing
  • End-to-end validation: Confirmed conversations are only created for spaces where the user is a participant

Files Changed

  • summarizer/grouping.py: Core bug fix and function signature update
  • tests/test_grouping.py: New test case, PII removal, and test updates

Impact

  • Security: ✅ Eliminates privacy leak where users could see conversations they weren't part of
  • Functionality: ✅ Preserves all existing functionality for legitimate conversations
  • Backward Compatibility: ✅ No breaking changes to the public API
  • Performance: ✅ No performance impact, same filtering logic but with correct user ID

Validation

The fix ensures that:

  • Conversations are only created for spaces where the authenticated user sent at least one message
  • Existing functionality for legitimate conversations continues to work unchanged
  • Cross-space contamination prevention remains intact
  • All privacy boundaries are properly enforced

This resolves the reported issue where conversations were being displayed to users who were not participants in those conversations.

…rticipation

* Modified `group_group_conversations` to accept `user_id` and check if the user participated in the conversation.
* Updated tests to reflect changes in user identifiers and ensure correct grouping behavior based on user participation.
* Added a test to prevent conversation leaks when the authenticated user is not a participant.
@ChristopherJHart ChristopherJHart added the bug Something isn't working label Aug 6, 2025
…upingBugFix

* Revised comments and docstrings for clarity and conciseness.
* Enhanced assertion messages to provide better context during test failures.
* Adjusted message content in assertions for improved readability.
@ChristopherJHart ChristopherJHart merged commit 42bf755 into main Aug 6, 2025
24 checks passed
@ChristopherJHart ChristopherJHart deleted the fix-non-participant-conversation-leak branch August 6, 2025 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant