feat: Enhance content extraction with type validation and security logging

Author: Johan Broberg

docs/prd/semantic-kernel-chat-history-api-tasks.md

@@ -90,14 +90,15 @@ The implementation is scoped as a minor version bump (1.0.0 to 1.1.0) since it a
 
 **Acceptance Criteria**:
 - [ ] Implement `_map_author_role(self, role: AuthorRole) -> str` that converts AuthorRole enum to lowercase string
-- [ ] Implement `_extract_content(self, message: ChatMessageContent) -> str` that extracts text content
+- [ ] Implement `_extract_content(self, message: ChatMessageContent) -> str` that extracts text content with type validation
 - [ ] Implement `_extract_or_generate_id(self, message: ChatMessageContent, index: int) -> str` that extracts ID from metadata or generates UUID
 - [ ] Implement `_extract_or_generate_timestamp(self, message: ChatMessageContent, index: int) -> datetime` that extracts timestamp or uses current UTC
 - [ ] Implement `_convert_single_sk_message(self, message: ChatMessageContent, index: int) -> Optional[ChatHistoryMessage]` that converts a single message
 - [ ] Implement `_convert_sk_messages_to_chat_history(self, messages: Sequence[ChatMessageContent]) -> List[ChatHistoryMessage]` that converts all messages
 - [ ] All methods include appropriate debug/warning logging
 - [ ] Methods handle None messages gracefully (skip with warning)
 - [ ] Methods handle empty/whitespace content gracefully (skip with warning)
+- [ ] **Security**: `_extract_content` returns empty string for unexpected content types (non-string) to prevent sensitive data exposure
 - [ ] Copyright header is present at file top
 
 **Technical Guidance**:
@@ -106,6 +107,7 @@ The implementation is scoped as a minor version bump (1.0.0 to 1.1.0) since it a
 - Use `message.metadata.get("id")` for existing IDs, `message.metadata.get("timestamp")` for timestamps
 - Support timestamp formats: datetime objects, Unix timestamps (int/float), ISO strings
 - Place these methods in a section marked `# PRIVATE HELPER METHODS - Message Conversion`
+- **Security**: In `_extract_content`, only return content if it's a string; for unexpected types (int, dict, objects), return empty string and log warning to avoid exposing sensitive data via `__str__` or `__repr__`
 
 **Dependencies**: Task 1 (imports must be in place)
 

docs/prd/semantic-kernel-chat-history-api.md

@@ -246,9 +246,11 @@ The following fields SHALL be mapped from Semantic Kernel `ChatMessageContent` t
 |-----------------------------|----------------------------|----------------|
 | `metadata.get("id")` or generated UUID | `id` | Extract from metadata or generate UUID |
 | `role` | `role` | Convert AuthorRole enum to lowercase string |
-| `content` | `content` | Direct copy (must be non-empty string) |
+| `content` | `content` | Direct copy (must be string type and non-empty) |
 | `metadata.get("timestamp")` or current UTC | `timestamp` | Extract from metadata or use current UTC time |
 
+**Security Note**: The `content` field MUST be a string type. If `content` is an unexpected type (not `str` or `None`), the method SHALL return empty string and log a warning. This prevents unintentionally exposing sensitive data that might be present in an object's `__str__` or `__repr__` methods.
+
 ### 4.4 Message Filtering Requirements
 
 Messages SHALL be filtered (skipped with warning log) if they are **invalid**:
@@ -257,6 +259,7 @@ Messages SHALL be filtered (skipped with warning log) if they are **invalid**:
 |-----------|--------|-------------|
 | Message is `None` | Cannot process null message | "Skipping null message at index {index}" |
 | Message `content` is None, empty, or whitespace-only | Content is required for chat history | "Skipping message at index {index} with empty content" |
+| Message `content` is unexpected type (not `str`) | Security: prevent data exposure from object stringification | "Unexpected content type '{type}' encountered. Returning empty string to avoid potential data exposure." |
 
 ### 4.5 Default Behavior
 
@@ -345,6 +348,7 @@ The implementation SHALL use the following types from the Semantic Kernel SDK:
 | Limit applied | INFO | "Applying limit of {limit} to {total} messages" |
 | Message skipped (null) | WARNING | "Skipping null message at index {index}" |
 | Message skipped (empty content) | WARNING | "Skipping message at index {index} with empty content" |
+| Unexpected content type | WARNING | "Unexpected content type '{type}' encountered. Returning empty string to avoid potential data exposure." |
 | All messages filtered | WARNING | "All messages were filtered out during conversion" |
 | ID generated | DEBUG | "Generated UUID {id} for message at index {index}" |
 | Timestamp generated | DEBUG | "Using current UTC time for message at index {index}" |

libraries/microsoft-agents-a365-tooling-extensions-semantickernel/microsoft_agents_a365/tooling/extensions/semantickernel/services/mcp_tool_registration_service.py

@@ -456,11 +456,26 @@ def _extract_content(self, message: ChatMessageContent) -> str:
             message: Semantic Kernel ChatMessageContent object.
 
         Returns:
-            Content string (may be empty).
+            Content string (may be empty). Returns empty string for unexpected
+            types to avoid unintentionally exposing sensitive data.
         """
-        if message.content is None:
+        content = message.content
+
+        if content is None:
             return ""
-        return str(message.content)
+
+        # If content is already a string, return it directly
+        if isinstance(content, str):
+            return content
+
+        # For unexpected types, log a warning and return empty string to avoid
+        # unintentionally stringifying objects that might contain sensitive data
+        content_type = type(content).__name__
+        self._logger.warning(
+            f"Unexpected content type '{content_type}' encountered. "
+            "Returning empty string to avoid potential data exposure."
+        )
+        return ""
 
     def _extract_or_generate_id(
         self,
@@ -517,8 +532,10 @@ def _extract_or_generate_timestamp(
                 elif isinstance(existing_timestamp, str):
                     try:
                         return datetime.fromisoformat(existing_timestamp.replace("Z", "+00:00"))
-                    except ValueError:
-                        pass
+                    except (ValueError, TypeError) as ex:
+                        self._logger.debug(
+                            f"Failed to parse timestamp '{existing_timestamp}' at index {index}: {ex}"
+                        )
 
         # Use current UTC time
         self._logger.debug(f"Using current UTC time for message at index {index}")

tests/tooling/extensions/semantickernel/services/test_send_chat_history.py

@@ -382,6 +382,28 @@ def test_extract_timestamp_from_created_at(self, service):
 
         assert result == timestamp
 
+    @pytest.mark.unit
+    def test_extract_timestamp_logs_on_invalid_iso_string(self, service, caplog):
+        """Test that invalid ISO string timestamp logs debug message and generates new timestamp."""
+        import logging
+
+        invalid_iso_string = "not-a-valid-timestamp"
+        message = MockChatMessageContent(
+            role=MockAuthorRole.USER,
+            content="Hello",
+            metadata={"timestamp": invalid_iso_string},
+        )
+
+        with caplog.at_level(logging.DEBUG):
+            result = service._extract_or_generate_timestamp(message, 5)
+
+        # Should generate a new timestamp (approximately now)
+        assert isinstance(result, datetime)
+        # Verify debug log was emitted
+        assert any("Failed to parse timestamp" in record.message for record in caplog.records)
+        assert any("not-a-valid-timestamp" in record.message for record in caplog.records)
+        assert any("index 5" in record.message for record in caplog.records)
+
 
 # =============================================================================
 # SUCCESS PATH TESTS (SP-01 to SP-06)
@@ -777,17 +799,46 @@ def test_extract_content_from_none(self, service):
         assert result == ""
 
     @pytest.mark.unit
-    def test_extract_content_converts_to_string(self, service):
-        """Test that content is converted to string."""
-        # Create a message with non-string content
+    def test_extract_content_returns_empty_for_unexpected_type(self, service, caplog):
+        """Test that unexpected content types return empty string for security."""
+        import logging
+
+        # Create a message with non-string content (integer)
         message = MockChatMessageContent(
             role=MockAuthorRole.USER,
-            content=12345,  # Integer
+            content=12345,  # Integer - unexpected type
         )
 
-        result = service._extract_content(message)
+        with caplog.at_level(logging.WARNING):
+            result = service._extract_content(message)
 
-        assert result == "12345"
+        # Should return empty string to avoid potential data exposure
+        assert result == ""
+
+        # Should log warning about unexpected type
+        assert any("Unexpected content type" in record.message for record in caplog.records)
+        assert any("int" in record.message for record in caplog.records)
+
+    @pytest.mark.unit
+    def test_extract_content_logs_warning_for_dict_type(self, service, caplog):
+        """Test that dict content type logs warning and returns empty string."""
+        import logging
+
+        # Create a message with dict content
+        message = MockChatMessageContent(
+            role=MockAuthorRole.USER,
+            content={"key": "value"},  # Dict - unexpected type
+        )
+
+        with caplog.at_level(logging.WARNING):
+            result = service._extract_content(message)
+
+        # Should return empty string to avoid potential data exposure
+        assert result == ""
+
+        # Should log warning about unexpected type
+        assert any("Unexpected content type" in record.message for record in caplog.records)
+        assert any("dict" in record.message for record in caplog.records)
 
 
 # =============================================================================