Content scanner also checks for thumbnails (#5911)

* scan also thumbnails for medias

* pr suggestion

ElementX/Sources/Other/SwiftUI/Views/ContentScanningView.swift

@@ -42,7 +42,7 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     }
     
     private let contentScannerService: ContentScannerServiceProtocol?
-    private let mediaSource: MediaSourceProxy?
+    private let mediaSources: [MediaSourceProxy]
     private let containerShowsFailure: Bool
     private let safeContent: () -> SafeContent
     private let scanningContent: () -> ScanningContent
@@ -55,6 +55,8 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     ///     scanning and the content is always rendered.
     ///   - mediaSource: The media source to scan. Passing `nil` renders the content, this
     ///     makes it convenient to wrap items whose media source is optional.
+    ///   - thumbnailSource: The thumbnail source to scan alongside the media source when the item
+    ///     also downloads a thumbnail. The content is flagged if either source is unsafe or fails.
     ///   - containerShowsFailure: Whether a failure is reported up the view hierarchy through
     ///     ``ContentScanningFailurePreferenceKey`` so that the surrounding container (such as the
     ///     message bubble) adopts the critical styling. Pass `false` when the failure should only
@@ -64,12 +66,13 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     ///   - unsafeContent: The presentation of the media when it can't be shown.
     init(contentScannerService: ContentScannerServiceProtocol?,
          mediaSource: MediaSourceProxy?,
+         thumbnailSource: MediaSourceProxy? = nil,
          containerShowsFailure: Bool = true,
          @ViewBuilder safeContent: @escaping () -> SafeContent,
          @ViewBuilder scanningContent: @escaping () -> ScanningContent,
          @ViewBuilder unsafeContent: @escaping (ContentScanningFailure) -> UnsafeContent) {
         self.contentScannerService = contentScannerService
-        self.mediaSource = mediaSource
+        mediaSources = [mediaSource, thumbnailSource].compactMap { $0 }
         self.containerShowsFailure = containerShowsFailure
         self.safeContent = safeContent
         self.scanningContent = scanningContent
@@ -79,7 +82,7 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     var body: some View {
         content
             .preference(key: ContentScanningFailurePreferenceKey.self, value: containerShowsFailure ? scanFailure : nil)
-            .task(id: mediaSource?.url.absoluteString) {
+            .task(id: mediaSources.map(\.url.absoluteString).joined(separator: "|")) {
                 await scan()
             }
     }
@@ -104,7 +107,7 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     }
     
     private var resolvedScanState: ScanState {
-        guard let contentScannerService, let mediaSource else {
+        guard let contentScannerService, !mediaSources.isEmpty else {
             return .safe
         }
         
@@ -113,7 +116,7 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
         }
         
         // Until the scan triggered by this view resolves, reflect any verdict already in the cache.
-        switch contentScannerService.scanResultFromSource(mediaSource) {
+        switch contentScannerService.scanResultFromSources(mediaSources) {
         case true: return .safe
         case false: return .unsafe(.notSafe)
         default: return .scanning
@@ -121,11 +124,11 @@ struct ContentScanningView<SafeContent: View, ScanningContent: View, UnsafeConte
     }
     
     private func scan() async {
-        guard let contentScannerService, let mediaSource else {
+        guard let contentScannerService, !mediaSources.isEmpty else {
             return
         }
         
-        switch await contentScannerService.loadScanResultFromSource(mediaSource) {
+        switch await contentScannerService.loadScanResultFromSources(mediaSources) {
         case .success(true):
             scanState = .safe
         case .success(false):

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift

@@ -137,17 +137,20 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
     }
     
     /// Scans the media when a content scanner is configured, returning whether it's safe to be downloaded
-    /// and previewed, reflecting the scan's progress and outcome in the current item.
+    /// and previewed, reflecting the scan's progress and outcome in the current item. Both the media and
+    /// its thumbnail are scanned as either being downloaded through the scanner can flag the media.
     private func checkSourceIsSafeIfNeeded(for mediaItem: TimelineMediaPreviewItem.Media, source: MediaSourceProxy) async -> Bool {
         guard let contentScannerService else { return true }
         
+        let sources = [source, mediaItem.thumbnailMediaSource].compactMap { $0 }
+        
         // Only reflect the scanning state when there's no cached verdict, so that
         // scanned items don't flash the scanning indicator when they're revisited.
-        if contentScannerService.scanResultFromSource(source) == nil {
+        if contentScannerService.scanResultFromSources(sources) == nil {
             setCurrentItem(.contentScan(.init(media: mediaItem, state: .scanning)))
         }
         
-        switch await contentScannerService.loadScanResultFromSource(source) {
+        switch await contentScannerService.loadScanResultFromSources(sources) {
         case .success(true):
             finishScan(with: .media(mediaItem), for: mediaItem)
             return true

ElementX/Sources/Screens/MediaEventsTimelineScreen/View/TimelineViews/FileMediaEventsTimelineView.swift

@@ -25,7 +25,8 @@ struct FileMediaEventsTimelineView: View {
                                      formattedCaption: timelineItem.content.formattedCaption,
                                      trailingReservedSize: timelineItem.trailingReservedSize,
                                      contentScannerService: context?.contentScannerService,
-                                     mediaSource: timelineItem.content.source)
+                                     mediaSource: timelineItem.content.source,
+                                     thumbnailSource: timelineItem.content.thumbnailSource)
             .accessibilityLabel(L10n.commonFile)
             .frame(maxWidth: .infinity, alignment: .leading)
             .bubbleBackground(isOutgoing: timelineItem.isOutgoing,

ElementX/Sources/Screens/MediaEventsTimelineScreen/View/TimelineViews/ImageMediaEventsTimelineView.swift

@@ -15,7 +15,8 @@ struct ImageMediaEventsTimelineView: View {
     
     var body: some View {
         ContentScanningView(contentScannerService: context?.contentScannerService,
-                            mediaSource: timelineItem.content.imageInfo.source) {
+                            mediaSource: timelineItem.content.imageInfo.source,
+                            thumbnailSource: timelineItem.content.thumbnailInfo?.source) {
             Color.clear // Let the image aspect fill in place
                 .aspectRatio(1, contentMode: .fill)
                 .overlay {

ElementX/Sources/Screens/MediaEventsTimelineScreen/View/TimelineViews/VideoMediaEventsTimelineView.swift

@@ -15,7 +15,8 @@ struct VideoMediaEventsTimelineView: View {
     
     var body: some View {
         ContentScanningView(contentScannerService: context?.contentScannerService,
-                            mediaSource: timelineItem.content.videoInfo.source) {
+                            mediaSource: timelineItem.content.videoInfo.source,
+                            thumbnailSource: timelineItem.content.thumbnailInfo?.source) {
             Color.clear // Let the image aspect fill in place
                 .aspectRatio(1, contentMode: .fill)
                 .overlay {

ElementX/Sources/Screens/Timeline/View/Replies/TimelineReplyView.swift

@@ -25,6 +25,7 @@ struct TimelineReplyView: View {
     var body: some View {
         ContentScanningView(contentScannerService: timelineContext?.contentScannerService,
                             mediaSource: scannedMediaSource,
+                            thumbnailSource: scannedThumbnailSource,
                             containerShowsFailure: false) {
             content
                 .roundedContainer(padding: 4,
@@ -63,6 +64,21 @@ struct TimelineReplyView: View {
         }
     }
     
+    /// The thumbnail source validated alongside ``scannedMediaSource`` when the replied to message has one.
+    private var scannedThumbnailSource: MediaSourceProxy? {
+        guard case .loaded(_, _, let eventContent) = timelineItemReplyDetails,
+              case .message(let message) = eventContent else {
+            return nil
+        }
+        
+        switch message {
+        case .file(let content): return content.thumbnailSource
+        case .image(let content): return content.thumbnailInfo?.source
+        case .video(let content): return content.thumbnailInfo?.source
+        default: return nil
+        }
+    }
+    
     @ViewBuilder
     private var content: some View {
         if let timelineItemReplyDetails {

ElementX/Sources/Screens/Timeline/View/TimelineItemViews/FileRoomTimelineView.swift

@@ -22,7 +22,8 @@ struct FileRoomTimelineView: View {
                                          trailingReservedSize: timelineItem.trailingReservedSize,
                                          shouldBoost: timelineItem.shouldBoost,
                                          contentScannerService: context?.contentScannerService,
-                                         mediaSource: timelineItem.content.source) {
+                                         mediaSource: timelineItem.content.source,
+                                         thumbnailSource: timelineItem.content.thumbnailSource) {
                 context?.send(viewAction: .mediaTapped(itemID: timelineItem.id))
             }
             .accessibilityLabel(L10n.commonFile)
@@ -42,6 +43,7 @@ struct MediaFileRoomTimelineContent: View {
     var isAudioFile = false
     var contentScannerService: ContentScannerServiceProtocol?
     var mediaSource: MediaSourceProxy?
+    var thumbnailSource: MediaSourceProxy?
     
     private var fileDescription: String {
         var fileDescription = "\(filename.validatedFileExtension.uppercased())"
@@ -60,7 +62,8 @@ struct MediaFileRoomTimelineContent: View {
     var body: some View {
         VStack(alignment: .leading, spacing: 8) {
             ContentScanningView(contentScannerService: contentScannerService,
-                                mediaSource: mediaSource) {
+                                mediaSource: mediaSource,
+                                thumbnailSource: thumbnailSource) {
                 if let onMediaTap {
                     filePreview(isScanning: false)
                         .onTapGesture(perform: onMediaTap)

ElementX/Sources/Screens/Timeline/View/TimelineItemViews/ImageRoomTimelineView.swift

@@ -24,7 +24,8 @@ struct ImageRoomTimelineView: View {
             // The caption sits 8pts below the content scanner failure placeholder, 4pts below the media.
             VStack(alignment: .leading, spacing: contentScanningFailure == nil ? 4 : 8) {
                 ContentScanningView(contentScannerService: context?.contentScannerService,
-                                    mediaSource: timelineItem.content.imageInfo.source) {
+                                    mediaSource: timelineItem.content.imageInfo.source,
+                                    thumbnailSource: timelineItem.content.thumbnailInfo?.source) {
                     loadableImage
                         .accessibilityElement(children: .ignore)
                         .accessibilityLabel(L10n.commonImage)

ElementX/Sources/Screens/Timeline/View/TimelineItemViews/VideoRoomTimelineView.swift

@@ -24,7 +24,8 @@ struct VideoRoomTimelineView: View {
             // The caption sits 8pts below the content scanner failure placeholder, 4pts below the media.
             VStack(alignment: .leading, spacing: contentScanningFailure == nil ? 4 : 8) {
                 ContentScanningView(contentScannerService: context?.contentScannerService,
-                                    mediaSource: timelineItem.content.videoInfo.source) {
+                                    mediaSource: timelineItem.content.videoInfo.source,
+                                    thumbnailSource: timelineItem.content.thumbnailInfo?.source) {
                     thumbnail
                         .timelineMediaFrame(imageInfo: timelineItem.content.thumbnailInfo)
                         .accessibilityElement(children: .ignore)

ElementX/Sources/Services/ContentScanner/ContentScannerProxy.swift

@@ -26,6 +26,10 @@ nonisolated struct ContentScannerProxy: ContentScannerProxyProtocol {
         do {
             let response = try await contentScanner.scan(client: client, mediaSource: mediaSource.underlyingSource)
             return .success(response.clean)
+        } catch ClientError.ContentScanner(.mcsMimeTypeForbidden, _), ClientError.ContentScanner(.mcsMediaNotClean, _) {
+            // The server flagged the media as unsafe (dangerous content or a forbidden mime type),
+            // which we treat as an unsafe verdict rather than a scan failure.
+            return .success(false)
         } catch {
             MXLog.error("Failed scanning media content: \(error)")
             return .failure(.sdkError(error))

ElementX/Sources/Services/ContentScanner/ContentScannerServiceProtocol.swift

@@ -25,3 +25,47 @@ nonisolated protocol ContentScannerServiceProtocol: Sendable {
     @discardableResult
     func loadScanResultFromSource(_ source: MediaSourceProxy) async -> Result<Bool, ContentScannerServiceError>
 }
+
+nonisolated extension ContentScannerServiceProtocol {
+    /// The combined cached verdict for several sources (e.g. a media source and its thumbnail, both
+    /// downloaded through the scanner): `false` if any is unsafe, `true` if all are safe, or `nil`
+    /// when any of them hasn't been scanned yet.
+    func scanResultFromSources(_ sources: [MediaSourceProxy]) -> Bool? {
+        let results = sources.map { scanResultFromSource($0) }
+        if results.contains(false) {
+            return false
+        }
+        if results.contains(where: { $0 == nil }) {
+            return nil
+        }
+        return true
+    }
+    
+    /// Scans several sources concurrently, resolving as soon as one comes back non-safe (unsafe or
+    /// failed) since a single failure is enough to flag the media - the remaining scans are then
+    /// ignored. Resolves as safe only when every source is safe.
+    @discardableResult
+    func loadScanResultFromSources(_ sources: [MediaSourceProxy]) async -> Result<Bool, ContentScannerServiceError> {
+        await withTaskGroup(of: Result<Bool, ContentScannerServiceError>.self) { group in
+            for source in sources {
+                group.addTask { await loadScanResultFromSource(source) }
+            }
+            
+            for await result in group where !result.isSafe {
+                group.cancelAll()
+                return result
+            }
+            
+            return .success(true)
+        }
+    }
+}
+
+private nonisolated extension Result where Success == Bool {
+    var isSafe: Bool {
+        if case .success(true) = self {
+            return true
+        }
+        return false
+    }
+}

UnitTests/Sources/ContentScannerServiceTests.swift

@@ -101,6 +101,66 @@ struct ContentScannerServiceTests {
         #expect(proxy.scanMediaSourceCallsCount == 1, "Concurrent loads for the same source should share a single scan.")
     }
     
+    // MARK: - Multiple sources
+    
+    @Test
+    func multipleSourcesAreSafeWhenAllSafe() async throws {
+        let proxy = ContentScannerProxyMock()
+        proxy.scanMediaSourceReturnValue = .success(true)
+        let service = ContentScannerService(contentScannerProxy: proxy)
+        
+        let result = await service.loadScanResultFromSources([makeSource("media"), makeSource("thumbnail")])
+        
+        #expect(try result.get() == true)
+    }
+    
+    @Test
+    func multipleSourcesAreUnsafeWhenAnySourceIsUnsafe() async throws {
+        let proxy = ContentScannerProxyMock()
+        let unsafeSource = makeSource("thumbnail")
+        proxy.scanMediaSourceClosure = { source in
+            source == unsafeSource ? .success(false) : .success(true)
+        }
+        let service = ContentScannerService(contentScannerProxy: proxy)
+        
+        // The media is safe but its thumbnail is unsafe, which is enough to flag the whole item.
+        let result = await service.loadScanResultFromSources([makeSource("media"), unsafeSource])
+        
+        #expect(try result.get() == false)
+    }
+    
+    @Test
+    func multipleSourcesFailWhenAnySourceFails() async {
+        let proxy = ContentScannerProxyMock()
+        let failingSource = makeSource("thumbnail")
+        proxy.scanMediaSourceClosure = { source in
+            source == failingSource ? .failure(.missingClient) : .success(true)
+        }
+        let service = ContentScannerService(contentScannerProxy: proxy)
+        
+        let result = await service.loadScanResultFromSources([makeSource("media"), failingSource])
+        
+        guard case .failure(.failedScanning) = result else {
+            Issue.record("A failure of any source should fail the combined scan.")
+            return
+        }
+    }
+    
+    @Test
+    func multipleSourcesSyncLookupCombinesVerdicts() async {
+        let proxy = ContentScannerProxyMock()
+        let service = ContentScannerService(contentScannerProxy: proxy)
+        
+        proxy.scanMediaSourceReturnValue = .success(true)
+        await service.loadScanResultFromSource(makeSource("safe"))
+        proxy.scanMediaSourceReturnValue = .success(false)
+        await service.loadScanResultFromSource(makeSource("unsafe"))
+        
+        #expect(service.scanResultFromSources([makeSource("safe")]) == true)
+        #expect(service.scanResultFromSources([makeSource("safe"), makeSource("unsafe")]) == false)
+        #expect(service.scanResultFromSources([makeSource("safe"), makeSource("unscanned")]) == nil)
+    }
+    
     // MARK: - Helpers
     
     private func makeSource(_ id: String) -> MediaSourceProxy {