Add scan check upon download

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java

@@ -133,7 +133,7 @@ public void eventHandlers(CdsRuntimeConfigurer configurer) {
           new EndTransactionMalwareScanRunner(null, null, malwareScanner, runtime);
       configurer.eventHandler(
           new ReadAttachmentsHandler(
-              attachmentService, new AttachmentStatusValidator(), scanRunner));
+              attachmentService, new AttachmentStatusValidator(), scanRunner, persistenceService));
     } else {
       logger.debug(
           "No application service is available. Application service event handlers will not be registered.");

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java

@@ -18,7 +18,9 @@
 import com.sap.cds.feature.attachments.service.AttachmentService;
 import com.sap.cds.feature.attachments.service.malware.AsyncMalwareScanExecutor;
 import com.sap.cds.ql.CQL;
+import com.sap.cds.ql.Update;
 import com.sap.cds.ql.cqn.CqnSelect;
+import com.sap.cds.ql.cqn.CqnUpdate;
 import com.sap.cds.ql.cqn.Path;
 import com.sap.cds.reflect.CdsAssociationType;
 import com.sap.cds.reflect.CdsElementDefinition;
@@ -32,7 +34,10 @@
 import com.sap.cds.services.handler.annotations.Before;
 import com.sap.cds.services.handler.annotations.HandlerOrder;
 import com.sap.cds.services.handler.annotations.ServiceName;
+import com.sap.cds.services.persistence.PersistenceService;
 import java.io.InputStream;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -45,29 +50,45 @@
 
 /**
  * The class {@link ReadAttachmentsHandler} is an event handler that is responsible for reading
- * attachments for entities. In the before read event, it modifies the CQN to include the content ID
- * and status. In the after read event, it adds a proxy for the stream of the attachments service to
- * the data. Only if the data are read the proxy forwards the request to the attachment service to
- * read the attachment. This is needed to have a filled stream in the data to enable the OData V4
- * adapter to enrich the data that a link to the content can be shown on the UI.
+ * attachments for entities. In the before read event, it modifies the CQN to include the content
+ * ID, status and scanned-at timestamp. In the after read event, it adds a proxy for the stream of
+ * the attachments service to the data. Only if the data are read the proxy forwards the request to
+ * the attachment service to read the attachment. This is needed to have a filled stream in the data
+ * to enable the OData V4 adapter to enrich the data that a link to the content can be shown on the
+ * UI.
+ *
+ * <p>Additionally, this handler implements rescan-on-download: if an attachment's last scan is
+ * older than {@link #RESCAN_THRESHOLD}, the handler transitions the attachment status to {@code
+ * SCANNING}, triggers an asynchronous malware rescan, and rejects the current download with a "not
+ * scanned" error. The client must retry the download after the rescan completes.
  */
 @ServiceName(value = "*", type = ApplicationService.class)
 public class ReadAttachmentsHandler implements EventHandler {
 
   private static final Logger logger = LoggerFactory.getLogger(ReadAttachmentsHandler.class);
 
+  /**
+   * The duration after which a previously scanned attachment is considered stale and must be
+   * rescanned on download, as recommended by the SAP Malware Scanning Service FAQ.
+   */
+  static final Duration RESCAN_THRESHOLD = Duration.ofDays(3);
+
   private final AttachmentService attachmentService;
   private final AttachmentStatusValidator statusValidator;
   private final AsyncMalwareScanExecutor scanExecutor;
+  private final PersistenceService persistenceService;
 
   public ReadAttachmentsHandler(
       AttachmentService attachmentService,
       AttachmentStatusValidator statusValidator,
-      AsyncMalwareScanExecutor scanExecutor) {
+      AsyncMalwareScanExecutor scanExecutor,
+      PersistenceService persistenceService) {
     this.attachmentService =
         requireNonNull(attachmentService, "attachmentService must not be null");
     this.statusValidator = requireNonNull(statusValidator, "statusValidator must not be null");
     this.scanExecutor = requireNonNull(scanExecutor, "scanExecutor must not be null");
+    this.persistenceService =
+        requireNonNull(persistenceService, "persistenceService must not be null");
   }
 
   @Before
@@ -153,9 +174,10 @@ private void verifyStatus(Path path, Attachments attachment) {
           "In verify status for content id {} and status {}",
           attachment.getContentId(),
           currentStatus);
-      if (StatusCode.UNSCANNED.equals(currentStatus)
-          || StatusCode.SCANNING.equals(currentStatus)
-          || currentStatus == null) {
+      if (needsScan(currentStatus, attachment.getScannedAt())) {
+        if (StatusCode.CLEAN.equals(currentStatus)) {
+          transitionToScanning(path.target().entity(), attachment);
+        }
         logger.debug(
             "Scanning content with ID {} for malware, has current status {}",
             attachment.getContentId(),
@@ -166,6 +188,39 @@ private void verifyStatus(Path path, Attachments attachment) {
     }
   }
 
+  private boolean needsScan(String status, Instant scannedAt) {
+    if (StatusCode.UNSCANNED.equals(status)
+        || StatusCode.SCANNING.equals(status)
+        || status == null) {
+      return true;
+    }
+    return StatusCode.CLEAN.equals(status) && isScanStale(scannedAt);
+  }
+
+  private boolean isScanStale(Instant scannedAt) {
+    return scannedAt == null || Instant.now().isAfter(scannedAt.plus(RESCAN_THRESHOLD));
+  }
+
+  private void transitionToScanning(CdsEntity entity, Attachments attachment) {
+    logger.debug(
+        "Attachment {} has stale scan (scannedAt={}), transitioning to SCANNING for rescan.",
+        attachment.getContentId(),
+        attachment.getScannedAt());
+
+    Attachments updateData = Attachments.create();
+    updateData.setStatus(StatusCode.SCANNING);
+
+    // Filter by contentId because primary keys are unavailable during content-only reads
+    // (areKeysEmpty returns true). This is consistent with DefaultAttachmentMalwareScanner.
+    CqnUpdate update =
+        Update.entity(entity)
+            .data(updateData)
+            .where(entry -> entry.get(Attachments.CONTENT_ID).eq(attachment.getContentId()));
+    persistenceService.run(update);
+
+    attachment.setStatus(StatusCode.SCANNING);
+  }
+
   private boolean areKeysEmpty(Map<String, Object> keys) {
     return keys.values().stream().allMatch(Objects::isNull);
   }

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEvent.java

@@ -66,6 +66,9 @@ public InputStream processEvent(
     eventContext.getChangeSetContext().register(createListener);
     path.target().values().put(Attachments.CONTENT_ID, result.contentId());
     path.target().values().put(Attachments.STATUS, result.status());
+    if (nonNull(result.scannedAt())) {
+      path.target().values().put(Attachments.SCANNED_AT, result.scannedAt());
+    }
     return result.isInternalStored() ? content : null;
   }
 

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifier.java

@@ -15,8 +15,8 @@
 import org.slf4j.LoggerFactory;
 
 /**
- * The class {@link BeforeReadItemsModifier} is a modifier that adds the content id field and status
- * code to the select items.
+ * The class {@link BeforeReadItemsModifier} is a modifier that adds the content id field, status
+ * code and scanned-at timestamp to the select items.
  */
 public class BeforeReadItemsModifier implements Modifier {
 
@@ -74,9 +74,10 @@ private List<CqnSelectListItem> processExpandedEntities(List<CqnSelectListItem>
   private void enhanceWithNewFieldForMediaAssociation(
       String association, List<CqnSelectListItem> list, List<CqnSelectListItem> listToEnhance) {
     if (isMediaAssociationAndNeedNewContentIdField(association, list)) {
-      logger.debug("Adding document id and status code to select items");
+      logger.debug("Adding document id, status code and scanned-at timestamp to select items");
       listToEnhance.add(CQL.get(Attachments.CONTENT_ID));
       listToEnhance.add(CQL.get(Attachments.STATUS));
+      listToEnhance.add(CQL.get(Attachments.SCANNED_AT));
     }
   }
 

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/AttachmentsServiceImpl.java

@@ -56,7 +56,8 @@ public AttachmentModificationResult createAttachment(CreateAttachmentInput input
     return new AttachmentModificationResult(
         Boolean.TRUE.equals(createContext.getIsInternalStored()),
         createContext.getContentId(),
-        createContext.getData().getStatus());
+        createContext.getData().getStatus(),
+        createContext.getData().getScannedAt());
   }
 
   @Override

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/model/service/AttachmentModificationResult.java

@@ -3,12 +3,15 @@
  */
 package com.sap.cds.feature.attachments.service.model.service;
 
+import java.time.Instant;
+
 /**
  * This record is used to store the result of the attachment modification.
  *
  * @param isInternalStored Indicates if the attachment is stored internally (in DB) or externally.
  * @param contentId The content id of the attachment.
  * @param status The status of the attachment.
+ * @param scannedAt The timestamp when the attachment was last scanned for malware.
  */
 public record AttachmentModificationResult(
-    boolean isInternalStored, String contentId, String status) {}
+    boolean isInternalStored, String contentId, String status, Instant scannedAt) {}

cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandlerTest.java

@@ -36,11 +36,14 @@
 import com.sap.cds.services.handler.annotations.Before;
 import com.sap.cds.services.handler.annotations.HandlerOrder;
 import com.sap.cds.services.handler.annotations.ServiceName;
+import com.sap.cds.services.persistence.PersistenceService;
 import com.sap.cds.services.runtime.CdsRuntime;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.List;
 import java.util.UUID;
 import org.junit.jupiter.api.BeforeAll;
@@ -61,6 +64,7 @@ class ReadAttachmentsHandlerTest {
   private AttachmentStatusValidator attachmentStatusValidator;
   private CdsReadEventContext readEventContext;
   private AsyncMalwareScanExecutor asyncMalwareScanExecutor;
+  private PersistenceService persistenceService;
 
   @BeforeAll
   static void classSetup() {
@@ -72,9 +76,13 @@ void setup() {
     attachmentService = mock(AttachmentService.class);
     attachmentStatusValidator = mock(AttachmentStatusValidator.class);
     asyncMalwareScanExecutor = mock(AsyncMalwareScanExecutor.class);
+    persistenceService = mock(PersistenceService.class);
     cut =
         new ReadAttachmentsHandler(
-            attachmentService, attachmentStatusValidator, asyncMalwareScanExecutor);
+            attachmentService,
+            attachmentStatusValidator,
+            asyncMalwareScanExecutor,
+            persistenceService);
 
     readEventContext = mock(CdsReadEventContext.class);
   }
@@ -164,6 +172,7 @@ void setAttachmentServiceCalled() throws IOException {
       attachment.setContentId("some ID");
       attachment.setContent(null);
       attachment.setStatus(StatusCode.CLEAN);
+      attachment.setScannedAt(Instant.now());
 
       cut.processAfter(readEventContext, List.of(attachment));
 
@@ -253,6 +262,98 @@ void scannerNotCalledForInfectedAttachments() {
     verifyNoInteractions(asyncMalwareScanExecutor);
   }
 
+  @Test
+  void scannerCalledForStaleCleanAttachment() {
+    mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
+    var attachment = Attachments.create();
+    attachment.setContentId("some ID");
+    attachment.setContent(mock(InputStream.class));
+    attachment.setStatus(StatusCode.CLEAN);
+    attachment.setScannedAt(Instant.now().minus(4, ChronoUnit.DAYS));
+    doThrow(AttachmentStatusException.class)
+        .when(attachmentStatusValidator)
+        .verifyStatus(StatusCode.SCANNING);
+
+    List<CdsData> attachments = List.of(attachment);
+    assertThrows(
+        AttachmentStatusException.class, () -> cut.processAfter(readEventContext, attachments));
+
+    verify(persistenceService).run(any(com.sap.cds.ql.cqn.CqnUpdate.class));
+    verify(asyncMalwareScanExecutor)
+        .scanAsync(readEventContext.getTarget(), attachment.getContentId());
+    assertThat(attachment.getStatus()).isEqualTo(StatusCode.SCANNING);
+  }
+
+  @Test
+  void scannerCalledForCleanAttachmentWithNullScannedAt() {
+    mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
+    var attachment = Attachments.create();
+    attachment.setContentId("some ID");
+    attachment.setContent(mock(InputStream.class));
+    attachment.setStatus(StatusCode.CLEAN);
+    attachment.setScannedAt(null);
+    doThrow(AttachmentStatusException.class)
+        .when(attachmentStatusValidator)
+        .verifyStatus(StatusCode.SCANNING);
+
+    List<CdsData> attachments = List.of(attachment);
+    assertThrows(
+        AttachmentStatusException.class, () -> cut.processAfter(readEventContext, attachments));
+
+    verify(persistenceService).run(any(com.sap.cds.ql.cqn.CqnUpdate.class));
+    verify(asyncMalwareScanExecutor)
+        .scanAsync(readEventContext.getTarget(), attachment.getContentId());
+    assertThat(attachment.getStatus()).isEqualTo(StatusCode.SCANNING);
+  }
+
+  @Test
+  void scannerNotCalledForFreshCleanAttachment() {
+    mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
+    var attachment = Attachments.create();
+    attachment.setContentId("some ID");
+    attachment.setContent(mock(InputStream.class));
+    attachment.setStatus(StatusCode.CLEAN);
+    attachment.setScannedAt(Instant.now().minus(1, ChronoUnit.DAYS));
+
+    cut.processAfter(readEventContext, List.of(attachment));
+
+    verifyNoInteractions(asyncMalwareScanExecutor);
+    verifyNoInteractions(persistenceService);
+    assertThat(attachment.getStatus()).isEqualTo(StatusCode.CLEAN);
+  }
+
+  @Test
+  void scannerNotCalledForCleanAttachmentScannedExactlyAtThreshold() {
+    mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
+    var attachment = Attachments.create();
+    attachment.setContentId("some ID");
+    attachment.setContent(mock(InputStream.class));
+    attachment.setStatus(StatusCode.CLEAN);
+    attachment.setScannedAt(
+        Instant.now().minus(ReadAttachmentsHandler.RESCAN_THRESHOLD).plusSeconds(60));
+
+    cut.processAfter(readEventContext, List.of(attachment));
+
+    verifyNoInteractions(asyncMalwareScanExecutor);
+    verifyNoInteractions(persistenceService);
+  }
+
+  @Test
+  void persistenceServiceNotCalledForUnscannedAttachments() {
+    mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class));
+    var attachment = Attachments.create();
+    attachment.setContentId("some ID");
+    attachment.setContent(mock(InputStream.class));
+    attachment.setStatus(StatusCode.UNSCANNED);
+
+    cut.processAfter(readEventContext, List.of(attachment));
+
+    verify(asyncMalwareScanExecutor)
+        .scanAsync(readEventContext.getTarget(), attachment.getContentId());
+    verify(attachmentStatusValidator).verifyStatus(StatusCode.UNSCANNED);
+    verifyNoInteractions(persistenceService);
+  }
+
   @Test
   void attachmentServiceNotCalledIfNoMediaType() {
     var eventItem = EventItems.create();

cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEventTest.java

@@ -92,7 +92,7 @@ void storageCalledWithAllFieldsFilledFromExistingData() {
     when(target.values()).thenReturn(attachment);
     when(target.keys()).thenReturn(Map.of("ID", attachment.getId(), "up__ID", "test"));
     when(attachmentService.createAttachment(any()))
-        .thenReturn(new AttachmentModificationResult(false, "id", "test"));
+        .thenReturn(new AttachmentModificationResult(false, "id", "test", null));
     var existingData = Attachments.create();
     existingData.setFileName("some file name");
     existingData.setMimeType("some mime type");
@@ -116,7 +116,7 @@ void resultFromServiceStoredInPath() {
     var attachment = Attachments.create();
     attachment.setId("test");
     var attachmentServiceResult =
-        new AttachmentModificationResult(false, "some document id", "test");
+        new AttachmentModificationResult(false, "some document id", "test", null);
     when(attachmentService.createAttachment(any())).thenReturn(attachmentServiceResult);
     when(target.values()).thenReturn(attachment);
 
@@ -134,7 +134,7 @@ void changesetIstRegistered() {
     var listener = mock(ChangeSetListener.class);
     when(listenerProvider.provideListener(contentId, runtime)).thenReturn(listener);
     when(attachmentService.createAttachment(any()))
-        .thenReturn(new AttachmentModificationResult(false, contentId, "test"));
+        .thenReturn(new AttachmentModificationResult(false, contentId, "test", null));
 
     cut.processEvent(path, null, Attachments.create(), eventContext);
 
@@ -154,7 +154,7 @@ void contentIsReturnedIfNotExternalStored(boolean isExternalStored) throws IOExc
     }
     when(target.values()).thenReturn(attachment);
     when(attachmentService.createAttachment(any()))
-        .thenReturn(new AttachmentModificationResult(isExternalStored, "id", "test"));
+        .thenReturn(new AttachmentModificationResult(isExternalStored, "id", "test", null));
 
     var result =
         cut.processEvent(path, attachment.getContent(), Attachments.create(), eventContext);
@@ -174,7 +174,7 @@ private Attachments prepareAndExecuteEventWithData() {
     when(target.values()).thenReturn(attachment);
     when(target.keys()).thenReturn(Map.of("ID", attachment.getId()));
     when(attachmentService.createAttachment(any()))
-        .thenReturn(new AttachmentModificationResult(false, "id", "test"));
+        .thenReturn(new AttachmentModificationResult(false, "id", "test", null));
 
     cut.processEvent(path, attachment.getContent(), Attachments.create(), eventContext);
     return attachment;