diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java index d749a42fb..c27fdb893 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/configuration/Registration.java +++ b/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."); diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java index 98f9c44ec..810152ec3 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java +++ b/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. + * + *

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 keys) { return keys.values().stream().allMatch(Objects::isNull); } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEvent.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEvent.java index 0b3295fb8..6d66f2793 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEvent.java +++ b/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; } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifier.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifier.java index c2890fef4..e5b16ab7b 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifier.java +++ b/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 processExpandedEntities(List private void enhanceWithNewFieldForMediaAssociation( String association, List list, List 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)); } } diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/AttachmentsServiceImpl.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/AttachmentsServiceImpl.java index 8fca05a57..37bf8469b 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/AttachmentsServiceImpl.java +++ b/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 diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/model/service/AttachmentModificationResult.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/model/service/AttachmentModificationResult.java index 0e1fd277a..0c62a4062 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/model/service/AttachmentModificationResult.java +++ b/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) {} diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandlerTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandlerTest.java index 588236b4f..0e70d7380 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandlerTest.java +++ b/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 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 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(); diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEventTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEventTest.java index 05f913939..df10cf9e8 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/modifyevents/CreateAttachmentEventTest.java +++ b/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; diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifierTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifierTest.java index d106cfea6..3c2444cc2 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifierTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/BeforeReadItemsModifierTest.java @@ -32,6 +32,7 @@ void expandSelectExtendsContentId() { cut = new BeforeReadItemsModifier(List.of("attachments")); runTestForExpand(cut, select, 1); + runTestForExpandScannedAt(cut, select, 1); } @Test @@ -111,6 +112,7 @@ void directSelectExtendsContentId() { Select.from(Attachment_.class).columns(Attachment_::ID, Attachment_::content); runTestForDirectSelect(select, 1); + runTestForDirectSelectScannedAt(select, 1); } @Test @@ -165,4 +167,35 @@ private void runTestForDirectSelect(CqnSelect select, int expectedFieldCount) { .count(); assertThat(count).isEqualTo(expectedFieldCount); } + + private void runTestForExpandScannedAt( + BeforeReadItemsModifier cut, CqnSelect select, int expectedFieldCount) { + List resultItems = cut.items(select.items()); + + var rootExpandedItem = + resultItems.stream().filter(CqnSelectListItem::isExpand).findAny().orElseThrow(); + var itemExpandedItem = + rootExpandedItem.asExpand().items().stream() + .filter(CqnSelectListItem::isExpand) + .findAny() + .orElseThrow(); + var count = + itemExpandedItem.asExpand().items().stream() + .filter( + item -> item.isRef() && item.asRef().displayName().equals(Attachments.SCANNED_AT)) + .count(); + assertThat(count).isEqualTo(expectedFieldCount); + } + + private void runTestForDirectSelectScannedAt(CqnSelect select, int expectedFieldCount) { + cut = new BeforeReadItemsModifier(List.of("")); + List resultItems = cut.items(select.items()); + + var count = + resultItems.stream() + .filter( + item -> item.isRef() && item.asRef().displayName().equals(Attachments.SCANNED_AT)) + .count(); + assertThat(count).isEqualTo(expectedFieldCount); + } } diff --git a/integration-tests/srv/src/main/java/com/sap/cds/feature/attachments/integrationtests/testhandler/TestPluginAttachmentsServiceHandler.java b/integration-tests/srv/src/main/java/com/sap/cds/feature/attachments/integrationtests/testhandler/TestPluginAttachmentsServiceHandler.java index a8c25829c..341002905 100644 --- a/integration-tests/srv/src/main/java/com/sap/cds/feature/attachments/integrationtests/testhandler/TestPluginAttachmentsServiceHandler.java +++ b/integration-tests/srv/src/main/java/com/sap/cds/feature/attachments/integrationtests/testhandler/TestPluginAttachmentsServiceHandler.java @@ -15,6 +15,7 @@ import com.sap.cds.services.handler.annotations.ServiceName; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -48,6 +49,7 @@ public void createAttachment(AttachmentCreateEventContext context) throws IOExce documents.put(contentId, context.getData().getContent().readAllBytes()); context.setContentId(contentId); context.getData().setStatus(StatusCode.CLEAN); + context.getData().setScannedAt(Instant.now()); context.setCompleted(); eventContextHolder.add( new EventContextHolder(AttachmentService.EVENT_CREATE_ATTACHMENT, context));