From aaabbedbe2e52e2a67931fc4670e9a1f2bcba85e Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Fri, 10 Apr 2026 17:39:29 +0200 Subject: [PATCH 1/5] Security hardening: credential scoping, S3 encryption, scanner logging --- .github/workflows/pipeline.yml | 28 +++++++++---------- .../DefaultAttachmentMalwareScanner.java | 10 +++++-- .../attachments/oss/client/AWSClient.java | 2 ++ .../attachments/oss/client/AWSClientTest.java | 28 +++++++++++++++++++ 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index e4a41fe2f..3ce1c86ad 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -2,20 +2,6 @@ name: Reusable Workflow env: MAVEN_VERSION: '3.9.12' - # Cloud storage environment variables (available to all jobs that need them) - ## AWS - AWS_S3_HOST: ${{ secrets.AWS_S3_HOST }} - AWS_S3_BUCKET: ${{ secrets.AWS_S3_BUCKET }} - AWS_S3_REGION: ${{ secrets.AWS_S3_REGION }} - AWS_S3_ACCESS_KEY_ID: ${{ secrets.AWS_S3_ACCESS_KEY_ID }} - AWS_S3_SECRET_ACCESS_KEY: ${{ secrets.AWS_S3_SECRET_ACCESS_KEY }} - ## Azure - AZURE_CONTAINER_URI: ${{ secrets.AZURE_CONTAINER_URI }} - AZURE_SAS_TOKEN: ${{ secrets.AZURE_SAS_TOKEN }} - ## GCP - GS_BASE_64_ENCODED_PRIVATE_KEY_DATA: ${{ secrets.GS_BASE_64_ENCODED_PRIVATE_KEY_DATA }} - GS_BUCKET: ${{ secrets.GS_BUCKET }} - GS_PROJECT_ID: ${{ secrets.GS_PROJECT_ID }} on: workflow_call: @@ -65,6 +51,20 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 needs: build + env: + ## AWS + AWS_S3_HOST: ${{ secrets.AWS_S3_HOST }} + AWS_S3_BUCKET: ${{ secrets.AWS_S3_BUCKET }} + AWS_S3_REGION: ${{ secrets.AWS_S3_REGION }} + AWS_S3_ACCESS_KEY_ID: ${{ secrets.AWS_S3_ACCESS_KEY_ID }} + AWS_S3_SECRET_ACCESS_KEY: ${{ secrets.AWS_S3_SECRET_ACCESS_KEY }} + ## Azure + AZURE_CONTAINER_URI: ${{ secrets.AZURE_CONTAINER_URI }} + AZURE_SAS_TOKEN: ${{ secrets.AZURE_SAS_TOKEN }} + ## GCP + GS_BASE_64_ENCODED_PRIVATE_KEY_DATA: ${{ secrets.GS_BASE_64_ENCODED_PRIVATE_KEY_DATA }} + GS_BUCKET: ${{ secrets.GS_BUCKET }} + GS_PROJECT_ID: ${{ secrets.GS_PROJECT_ID }} strategy: fail-fast: false matrix: diff --git a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java index 7b53c6484..8f22ad570 100644 --- a/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java +++ b/cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/service/malware/DefaultAttachmentMalwareScanner.java @@ -94,7 +94,7 @@ private MalwareScanResultStatus findAndScanAttachments( return selectionResults.stream() .filter(result -> validateAndFilter(result, contentId)) .findFirst() - .map(result -> scanDocument(result.result().single(Attachments.class))) + .map(result -> scanDocument(result.result().single(Attachments.class), result.entity)) .orElse(null); } @@ -158,7 +158,7 @@ private Result readData(String contentId, CdsEntity entity) { return result; } - private MalwareScanResultStatus scanDocument(Attachments attachment) { + private MalwareScanResultStatus scanDocument(Attachments attachment, CdsEntity attachmentEntity) { if (malwareScanClient != null) { try { InputStream content = @@ -168,7 +168,11 @@ private MalwareScanResultStatus scanDocument(Attachments attachment) { logger.debug("Start scanning attachment {}.", attachment.getContentId()); return malwareScanClient.scanContent(content); } catch (RuntimeException e) { - logger.error("Error while scanning attachment {}.", attachment.getContentId(), e); + logger.error( + "Error while scanning attachment {} in entity {}.", + attachment.getContentId(), + attachmentEntity.getQualifiedName(), + e); return MalwareScanResultStatus.FAILED; } } diff --git a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java index 9a7d592e8..81fc9cb0e 100644 --- a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java +++ b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java @@ -32,6 +32,7 @@ import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectResponse; import software.amazon.awssdk.services.s3.model.S3Error; +import software.amazon.awssdk.services.s3.model.ServerSideEncryption; public class AWSClient implements OSClient { private final S3Client s3Client; @@ -89,6 +90,7 @@ public Future uploadContent( .bucket(this.bucketName) .key(completeFileName) .contentType(contentType) + .serverSideEncryption(ServerSideEncryption.AES256) .build(); CompletableFuture putFuture = this.s3AsyncClient.putObject(putRequest, body); diff --git a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java index df25d4fa2..12caffb99 100644 --- a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java +++ b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java @@ -4,6 +4,7 @@ package com.sap.cds.feature.attachments.oss.client; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -42,6 +43,7 @@ import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectResponse; import software.amazon.awssdk.services.s3.model.S3Object; +import software.amazon.awssdk.services.s3.model.ServerSideEncryption; class AWSClientTest { ExecutorService executor = Executors.newCachedThreadPool(); @@ -88,6 +90,32 @@ void testUploadContent() throws Exception { .get(); } + @Test + void testUploadContentSetsServerSideEncryption() throws Exception { + S3AsyncClient mockAsyncClient = mock(S3AsyncClient.class); + AWSClient awsClient = new AWSClient(mock(S3Client.class), mockAsyncClient, "bucket", executor); + + PutObjectResponse mockPutRes = mock(PutObjectResponse.class); + SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class); + when(mockHttpRes.isSuccessful()).thenReturn(true); + when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes); + CompletableFuture successFuture = + CompletableFuture.completedFuture(mockPutRes); + when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class))) + .thenReturn(successFuture); + + awsClient + .uploadContent(new ByteArrayInputStream("test".getBytes()), "test.txt", "text/plain") + .get(); + + verify(mockAsyncClient) + .putObject( + argThat( + (PutObjectRequest req) -> + req.serverSideEncryption() == ServerSideEncryption.AES256), + any(AsyncRequestBody.class)); + } + @Test void testDeleteContent() { S3Client mockS3Client = mock(S3Client.class); From f7bd3f504654d9ccb5a5c95989109d0124eb3c17 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Fri, 10 Apr 2026 17:40:57 +0200 Subject: [PATCH 2/5] remove unused import --- .../sap/cds/feature/attachments/oss/client/AWSClientTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java index 12caffb99..c1f49e043 100644 --- a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java +++ b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java @@ -4,7 +4,6 @@ package com.sap.cds.feature.attachments.oss.client; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; From ff40b78b5091fe40a4121bd7fcda9708cca9ca68 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Fri, 10 Apr 2026 17:45:16 +0200 Subject: [PATCH 3/5] Extract configureSuccessfulPut helper in AWSClientTest --- .../attachments/oss/client/AWSClientTest.java | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java index c1f49e043..6ad31eaf4 100644 --- a/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java +++ b/storage-targets/cds-feature-attachments-oss/src/test/java/com/sap/cds/feature/attachments/oss/client/AWSClientTest.java @@ -74,15 +74,7 @@ void testReadContent() throws Exception { void testUploadContent() throws Exception { S3AsyncClient mockAsyncClient = mock(S3AsyncClient.class); AWSClient awsClient = new AWSClient(mock(S3Client.class), mockAsyncClient, "bucket", executor); - - PutObjectResponse mockPutRes = mock(PutObjectResponse.class); - SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class); - when(mockHttpRes.isSuccessful()).thenReturn(true); - when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes); - CompletableFuture successFuture = - CompletableFuture.completedFuture(mockPutRes); - when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class))) - .thenReturn(successFuture); + configureSuccessfulPut(mockAsyncClient); awsClient .uploadContent(new ByteArrayInputStream("test".getBytes()), "test.txt", "text/plain") @@ -93,15 +85,7 @@ void testUploadContent() throws Exception { void testUploadContentSetsServerSideEncryption() throws Exception { S3AsyncClient mockAsyncClient = mock(S3AsyncClient.class); AWSClient awsClient = new AWSClient(mock(S3Client.class), mockAsyncClient, "bucket", executor); - - PutObjectResponse mockPutRes = mock(PutObjectResponse.class); - SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class); - when(mockHttpRes.isSuccessful()).thenReturn(true); - when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes); - CompletableFuture successFuture = - CompletableFuture.completedFuture(mockPutRes); - when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class))) - .thenReturn(successFuture); + configureSuccessfulPut(mockAsyncClient); awsClient .uploadContent(new ByteArrayInputStream("test".getBytes()), "test.txt", "text/plain") @@ -314,6 +298,17 @@ void testDeleteContentByPrefixWithPagination() throws Exception { verify(mockS3Client, times(2)).deleteObjects(any(DeleteObjectsRequest.class)); } + private void configureSuccessfulPut(S3AsyncClient mockAsyncClient) { + PutObjectResponse mockPutRes = mock(PutObjectResponse.class); + SdkHttpResponse mockHttpRes = mock(SdkHttpResponse.class); + when(mockHttpRes.isSuccessful()).thenReturn(true); + when(mockPutRes.sdkHttpResponse()).thenReturn(mockHttpRes); + CompletableFuture successFuture = + CompletableFuture.completedFuture(mockPutRes); + when(mockAsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class))) + .thenReturn(successFuture); + } + private ServiceBinding getDummyBinding() { ServiceBinding binding = mock(ServiceBinding.class); HashMap creds = new HashMap<>(); From 48e9060364da4ea7cbbb79a19db0dea1d2aa09f7 Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 13 Apr 2026 12:01:07 +0200 Subject: [PATCH 4/5] add comment --- .../com/sap/cds/feature/attachments/oss/client/AWSClient.java | 1 + 1 file changed, 1 insertion(+) diff --git a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java index 81fc9cb0e..fc028e93e 100644 --- a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java +++ b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java @@ -90,6 +90,7 @@ public Future uploadContent( .bucket(this.bucketName) .key(completeFileName) .contentType(contentType) + // Azure and Google Cloud Storage encrypt at rest by default; S3 requires explicit opt-in .serverSideEncryption(ServerSideEncryption.AES256) .build(); From b86a21133e876ab5a6278aa20389e04a28561f5b Mon Sep 17 00:00:00 2001 From: Marvin Lindner Date: Mon, 13 Apr 2026 12:07:51 +0200 Subject: [PATCH 5/5] spotless --- .../com/sap/cds/feature/attachments/oss/client/AWSClient.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java index fc028e93e..a7690ab05 100644 --- a/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java +++ b/storage-targets/cds-feature-attachments-oss/src/main/java/com/sap/cds/feature/attachments/oss/client/AWSClient.java @@ -90,7 +90,8 @@ public Future uploadContent( .bucket(this.bucketName) .key(completeFileName) .contentType(contentType) - // Azure and Google Cloud Storage encrypt at rest by default; S3 requires explicit opt-in + // Azure and Google Cloud Storage encrypt at rest by default; S3 requires explicit + // opt-in .serverSideEncryption(ServerSideEncryption.AES256) .build();