From 80ac5c7bfb19d4f14d613f79f95652d107d507bb Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 21 Jul 2026 12:48:34 +0200 Subject: [PATCH 1/2] fix: enforce rescan-on-download for all content read shapes The stale-clean rescan-on-download check only ran for content-only reads (empty target keys). Keyed reads that stream content went through LazyProxyInputStream, which validated only the stored status and skipped the scannedAt freshness check, so a stale clean attachment could be downloaded through a keyed path without triggering a rescan. The status and freshness policy is now encapsulated in a ContentReadGuard that LazyProxyInputStream invokes before serving bytes, so every read shape enforces the same rescan-on-download and status validation. The eager content-only check is retained so those responses are still blocked before serialization. --- .../ReadAttachmentsHandler.java | 42 ++++++++++++------- .../readhelper/ContentReadGuard.java | 25 +++++++++++ .../readhelper/LazyProxyInputStream.java | 15 +++---- 3 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/ContentReadGuard.java 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 0848702cb..758969c49 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 @@ -115,11 +115,16 @@ void processAfter(CdsReadEventContext context, List data) { InputStream content = attachment.getContent(); if (nonNull(attachment.getContentId())) { verifyStatus(path, attachment); + CdsEntity attachmentEntity = path.target().entity(); Supplier supplier = nonNull(content) ? () -> content : () -> attachmentService.readAttachment(attachment.getContentId()); - return new LazyProxyInputStream(supplier, statusValidator, attachment.getStatus()); + // Enforce the status/freshness policy before bytes are served for every read shape, + // including keyed reads that stream content (content-only reads are additionally + // guarded eagerly above via verifyStatus). + return new LazyProxyInputStream( + supplier, () -> enforceReadPolicy(attachmentEntity, attachment)); } else { return value; } @@ -133,23 +138,32 @@ void processAfter(CdsReadEventContext context, List data) { private void verifyStatus(Path path, Attachments attachment) { if (areKeysEmpty(path.target().keys())) { - String currentStatus = attachment.getStatus(); + enforceReadPolicy(path.target().entity(), attachment); + } + } + + /** + * Enforces the malware scan status and freshness policy for the given attachment: triggers a + * rescan when the attachment is unscanned, scanning, or a stale clean, and validates that the + * (possibly updated) status permits serving the content. + */ + private void enforceReadPolicy(CdsEntity entity, Attachments attachment) { + String currentStatus = attachment.getStatus(); + logger.debug( + "In verify status for content id {} and status {}", + attachment.getContentId(), + currentStatus); + if (scannerAvailable && needsScan(currentStatus, attachment.getScannedAt())) { + if (StatusCode.CLEAN.equals(currentStatus)) { + transitionToScanning(entity, attachment); + } logger.debug( - "In verify status for content id {} and status {}", + "Scanning content with ID {} for malware, has current status {}", attachment.getContentId(), currentStatus); - if (scannerAvailable && 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(), - currentStatus); - scanExecutor.scanAsync(path.target().entity(), attachment.getContentId()); - } - statusValidator.verifyStatus(attachment.getStatus()); + scanExecutor.scanAsync(entity, attachment.getContentId()); } + statusValidator.verifyStatus(attachment.getStatus()); } private boolean needsScan(String status, Instant scannedAt) { diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/ContentReadGuard.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/ContentReadGuard.java new file mode 100644 index 000000000..31287eb44 --- /dev/null +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/ContentReadGuard.java @@ -0,0 +1,25 @@ +/* + * © 2026 SAP SE or an SAP affiliate company and cds-feature-attachments contributors. + */ +package com.sap.cds.feature.attachments.handler.applicationservice.readhelper; + +/** + * Guard invoked by {@link LazyProxyInputStream} right before attachment content bytes are served. + * + *

It enforces the malware scan status and freshness policy (rescan-on-download) for the content + * that is about to be read, regardless of the read shape (content-only {@code $value} reads as well + * as keyed reads that stream the content). Implementations throw an {@link + * AttachmentStatusException} to block the download when the content is not clean or must be + * rescanned first. + */ +@FunctionalInterface +public interface ContentReadGuard { + + /** + * Enforces the status and freshness policy before content is served. + * + * @throws AttachmentStatusException if the content must not be served (e.g. not clean or a rescan + * has been triggered) + */ + void verify(); +} diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java index 850b588f1..9f7deaec2 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStream.java @@ -12,23 +12,20 @@ /** * The class {@link LazyProxyInputStream} is a lazy proxy for an {@link InputStream}. The class is * used to create a proxy for an {@link InputStream} that is not yet available. Before the {@link - * InputStream} is accessed, the status of the attachment is validated. + * InputStream} is accessed, the {@link ContentReadGuard} enforces the attachment status and + * freshness policy. */ public class LazyProxyInputStream extends InputStream { private static final Logger logger = LoggerFactory.getLogger(LazyProxyInputStream.class); private final Supplier inputStreamSupplier; - private final AttachmentStatusValidator attachmentStatusValidator; - private final String status; + private final ContentReadGuard contentReadGuard; private InputStream delegate; public LazyProxyInputStream( - Supplier inputStreamSupplier, - AttachmentStatusValidator attachmentStatusValidator, - String status) { + Supplier inputStreamSupplier, ContentReadGuard contentReadGuard) { this.inputStreamSupplier = inputStreamSupplier; - this.attachmentStatusValidator = attachmentStatusValidator; - this.status = status; + this.contentReadGuard = contentReadGuard; } @Override @@ -58,7 +55,7 @@ public void close() throws IOException { } private InputStream getDelegate() { - attachmentStatusValidator.verifyStatus(status); + contentReadGuard.verify(); if (delegate == null) { logger.debug("Creating delegate input stream"); From 5a5d100bfa8d36f6bab9646e83a444c8d6b933e9 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Tue, 21 Jul 2026 12:49:15 +0200 Subject: [PATCH 2/2] test: cover rescan enforcement on keyed content reads Adds ReadAttachmentsHandler tests asserting that a keyed read of a stale-clean attachment triggers the rescan and blocks the download when the content is actually read, and that a keyed infected attachment is blocked on content read. Updates LazyProxyInputStreamTest for the new ContentReadGuard. --- .../ReadAttachmentsHandlerTest.java | 49 +++++++++++++++++++ .../readhelper/LazyProxyInputStreamTest.java | 40 ++++++--------- 2 files changed, 65 insertions(+), 24 deletions(-) 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 524c52c85..9af225587 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 @@ -377,6 +377,55 @@ void statusNotVerifiedIfNotOnlyContentIsRequested() { verifyNoInteractions(attachmentStatusValidator); } + @Test + void keyedStaleCleanAttachmentIsRescannedWhenContentIsRead() throws IOException { + mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class)); + var attachment = Attachments.create(); + attachment.setId(UUID.randomUUID().toString()); + 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); + + cut.processAfter(readEventContext, List.of(attachment)); + + // keyed read: nothing is scanned or validated eagerly + verifyNoInteractions(asyncMalwareScanExecutor); + verifyNoInteractions(persistenceService); + + // reading the content enforces the freshness policy: rescan is triggered and download blocked + var content = attachment.getContent(); + assertThat(content).isInstanceOf(LazyProxyInputStream.class); + assertThrows(AttachmentStatusException.class, content::readAllBytes); + 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 keyedInfectedAttachmentIsBlockedWhenContentIsRead() { + mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class)); + var attachment = Attachments.create(); + attachment.setId(UUID.randomUUID().toString()); + attachment.setContentId("some ID"); + attachment.setContent(mock(InputStream.class)); + attachment.setStatus(StatusCode.INFECTED); + doThrow(AttachmentStatusException.class) + .when(attachmentStatusValidator) + .verifyStatus(StatusCode.INFECTED); + + cut.processAfter(readEventContext, List.of(attachment)); + + var content = attachment.getContent(); + assertThat(content).isInstanceOf(LazyProxyInputStream.class); + verifyNoInteractions(attachmentService); + assertThrows(AttachmentStatusException.class, content::readAllBytes); + } + @Test void emptyContentIdAndEmptyContentReturnNullContent() { mockEventContext(Attachment_.CDS_NAME, mock(CqnSelect.class)); diff --git a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java index 0413e96ac..07586f643 100644 --- a/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java +++ b/cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/readhelper/LazyProxyInputStreamTest.java @@ -14,7 +14,6 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; -import com.sap.cds.feature.attachments.generated.cds4j.sap.attachments.StatusCode; import com.sap.cds.feature.attachments.service.AttachmentService; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; @@ -22,32 +21,27 @@ import java.nio.charset.StandardCharsets; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; class LazyProxyInputStreamTest { private LazyProxyInputStream cut; private InputStream inputStream; private AttachmentService attachmentService; - private AttachmentStatusValidator attachmentStatusValidator; + private ContentReadGuard contentReadGuard; @BeforeEach void setup() { inputStream = mock(InputStream.class); attachmentService = mock(AttachmentService.class); - attachmentStatusValidator = mock(AttachmentStatusValidator.class); + contentReadGuard = mock(ContentReadGuard.class); when(attachmentService.readAttachment(any())).thenReturn(inputStream); - cut = - new LazyProxyInputStream( - () -> attachmentService.readAttachment(any()), - attachmentStatusValidator, - StatusCode.CLEAN); + cut = new LazyProxyInputStream(() -> attachmentService.readAttachment(any()), contentReadGuard); } @Test void noMethodCallNoStreamAccess() { verifyNoInteractions(attachmentService); + verifyNoInteractions(contentReadGuard); } @Test @@ -115,26 +109,24 @@ void closeCallsInputStream() throws IOException { verify(attachmentService).readAttachment(any()); } - @ParameterizedTest - @ValueSource(strings = {StatusCode.UNSCANNED, StatusCode.INFECTED}) - void exceptionIfWrongStatus(String status) { - doThrow(AttachmentStatusException.class).when(attachmentStatusValidator).verifyStatus(status); + @Test + void guardIsVerifiedBeforeContentIsServed() throws IOException { + cut.read(); + + verify(contentReadGuard).verify(); + verify(attachmentService).readAttachment(any()); + } - cut = - new LazyProxyInputStream( - () -> attachmentService.readAttachment(any()), attachmentStatusValidator, status); + @Test + void exceptionIfGuardBlocksRead() { + doThrow(AttachmentStatusException.class).when(contentReadGuard).verify(); assertThrows(AttachmentStatusException.class, () -> cut.read()); + verifyNoInteractions(attachmentService); } @Test - void noExceptionIfCorrectStatus() { - cut = - new LazyProxyInputStream( - () -> attachmentService.readAttachment(any()), - attachmentStatusValidator, - StatusCode.CLEAN); - + void noExceptionIfGuardPasses() { assertDoesNotThrow(() -> cut.read()); } }