From 99da2adf2764e8901b4ec9daf1ec62d8d26e8784 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 09:33:50 +0100 Subject: [PATCH 01/22] feat: rejection events --- lib/generic-handlers.js | 29 +- lib/plugin.js | 2 +- tests/unit/rejectionEvents.test.js | 313 ++++++++++++++++++ tests/unit/validateAttachmentMimeType.test.js | 20 +- 4 files changed, 351 insertions(+), 13 deletions(-) create mode 100644 tests/unit/rejectionEvents.test.js diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 5fb88403..5549168e 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -151,6 +151,13 @@ async function validateAttachment(req) { if (scanEnabled) { if (status !== "Clean") { + AttachmentsSrv.emit("AttachmentDownloadRejected", { + target: req.target.name, + keys: { ID: attachmentId }, + status, + }).catch((err) => + LOG.error("Failed to emit AttachmentDownloadRejected", err), + ) req.reject(403, "UnableToDownloadAttachmentScanStatusNotClean") } @@ -263,6 +270,15 @@ async function validateAttachmentSize(req, validateContentLength = false) { .from(req.target) .where({ up__ID: req.data.up__ID }) + const AttachmentsSrv = await cds.connect.to("attachments") + AttachmentsSrv.emit("AttachmentSizeExceeded", { + target: req.target.name, + keys: req.data.ID ? { ID: req.data.ID } : {}, + filename: attachmentRef?.filename ?? "n/a", + fileSize: length, + maxFileSize, + }).catch((err) => LOG.error("Failed to emit AttachmentSizeExceeded", err)) + req.reject({ status: 413, message: "AttachmentSizeExceeded", @@ -280,7 +296,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { * Validates the attachment mime type against acceptable media types * @param {import('@sap/cds').Request} req - The request object */ -function validateAttachmentMimeType(req) { +async function validateAttachmentMimeType(req) { if (!req.target?._attachments.isAttachmentsEntity || !req.data.content) return false @@ -289,6 +305,14 @@ function validateAttachmentMimeType(req) { const acceptableMediaTypes = req.target.elements.content["@Core.AcceptableMediaTypes"] || "*/*" if (!checkMimeTypeMatch(acceptableMediaTypes, mimeType)) { + const AttachmentsSrv = await cds.connect.to("attachments") + AttachmentsSrv.emit("AttachmentUploadRejected", { + target: req.target.name, + keys: req.data.ID ? { ID: req.data.ID } : {}, + filename: req.data.filename, + mimeType, + reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, + }).catch((err) => LOG.error("Failed to emit AttachmentUploadRejected", err)) req.reject(400, "AttachmentMimeTypeDisallowed", { mimeType: mimeType, }) @@ -300,7 +324,8 @@ function validateAttachmentMimeType(req) { async function validateAndInsertAttachmentFromDBHandler(data, target, req) { if (!data.content) return - if (!validateAttachmentMimeType({ data, target, reject: req.reject })) return + if (!(await validateAttachmentMimeType({ data, target, reject: req.reject }))) + return if ( !(await validateAttachmentSize({ data, target, reject: req.reject }, true)) ) diff --git a/lib/plugin.js b/lib/plugin.js index 1ae1c3d5..ce83a384 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -188,7 +188,7 @@ cds.ApplicationService.handle_attachments = cds.service.impl(async function () { } } // Validate mimeType against @Core.AcceptableMediaTypes before uploading content - if (!validateAttachmentMimeType(req)) return + if (!(await validateAttachmentMimeType(req))) return const AttachmentsSrv = await cds.connect.to("attachments") try { return await AttachmentsSrv.put(req.target, req.data) diff --git a/tests/unit/rejectionEvents.test.js b/tests/unit/rejectionEvents.test.js new file mode 100644 index 00000000..94fdd8f8 --- /dev/null +++ b/tests/unit/rejectionEvents.test.js @@ -0,0 +1,313 @@ +require("../../lib/csn-runtime-extension") +const cds = require("@sap/cds") +const { join } = cds.utils.path +const app = join(__dirname, "../incidents-app") +cds.test(app) + +const { + validateAttachmentMimeType, + validateAttachmentSize, +} = require("../../lib/generic-handlers") + +let attachmentsSvc +let originalConnectTo + +beforeEach(() => { + jest.restoreAllMocks() + + attachmentsSvc = { + emit: jest.fn().mockResolvedValue(undefined), + getStatus: jest.fn(), + } + originalConnectTo = cds.connect.to + cds.connect.to = jest.fn().mockImplementation((name) => { + if (name === "attachments") return Promise.resolve(attachmentsSvc) + return originalConnectTo.call(cds.connect, name) + }) +}) + +afterEach(() => { + cds.connect.to = originalConnectTo +}) + +describe("AttachmentUploadRejected event", () => { + it("should emit when MIME type is rejected", async () => { + const req = { + target: { + name: "TestService.Attachments", + _attachments: { isAttachmentsEntity: true }, + elements: { + content: { + "@Core.AcceptableMediaTypes": ["image/jpeg", "image/png"], + }, + }, + }, + data: { + content: "test content", + mimeType: "text/plain", + ID: "att-123", + filename: "notes.txt", + }, + reject: jest.fn(), + } + + await validateAttachmentMimeType(req) + + expect(req.reject).toHaveBeenCalledWith( + 400, + "AttachmentMimeTypeDisallowed", + { + mimeType: "text/plain", + }, + ) + + // Wait for the fire-and-forget promise to resolve + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(attachmentsSvc.emit).toHaveBeenCalledWith( + "AttachmentUploadRejected", + expect.objectContaining({ + target: "TestService.Attachments", + keys: { ID: "att-123" }, + filename: "notes.txt", + mimeType: "text/plain", + reason: expect.stringContaining("@Core.AcceptableMediaTypes"), + }), + ) + }) + + it("should not emit when MIME type is allowed", async () => { + const req = { + target: { + name: "TestService.Attachments", + _attachments: { isAttachmentsEntity: true }, + elements: { + content: { + "@Core.AcceptableMediaTypes": ["image/jpeg"], + }, + }, + }, + data: { + content: "test content", + mimeType: "image/jpeg", + ID: "att-123", + filename: "photo.jpg", + }, + reject: jest.fn(), + } + + const result = await validateAttachmentMimeType(req) + + expect(result).toBe(true) + expect(req.reject).not.toHaveBeenCalled() + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(attachmentsSvc.emit).not.toHaveBeenCalled() + }) + + it("should still reject when event handler fails", async () => { + attachmentsSvc.emit.mockRejectedValue(new Error("handler error")) + + const req = { + target: { + name: "TestService.Attachments", + _attachments: { isAttachmentsEntity: true }, + elements: { + content: { + "@Core.AcceptableMediaTypes": ["image/jpeg"], + }, + }, + }, + data: { + content: "test content", + mimeType: "text/plain", + ID: "att-123", + filename: "notes.txt", + }, + reject: jest.fn(), + } + + const result = await validateAttachmentMimeType(req) + + expect(result).toBe(false) + expect(req.reject).toHaveBeenCalledWith( + 400, + "AttachmentMimeTypeDisallowed", + { + mimeType: "text/plain", + }, + ) + }) +}) + +describe("AttachmentSizeExceeded event", () => { + it("should emit when file size exceeds the limit", async () => { + // Use maximumSizeAttachments which has @Validation.Maximum: '5MB' + const target = + cds.model.definitions["AdminService.Incidents.maximumSizeAttachments"] + + const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } + await INSERT.into(target).entries({ + ...keys, + filename: "large-file.pdf", + status: "Scanning", + }) + + const req = { + target, + data: { + content: { pause: jest.fn() }, + up__ID: keys.up__ID, + ID: keys.ID, + }, + headers: { "content-length": "999999999999" }, + reject: jest.fn(), + } + + await validateAttachmentSize(req) + + expect(req.reject).toHaveBeenCalledWith( + expect.objectContaining({ + status: 413, + message: "AttachmentSizeExceeded", + }), + ) + + // Wait for fire-and-forget promise + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(attachmentsSvc.emit).toHaveBeenCalledWith( + "AttachmentSizeExceeded", + expect.objectContaining({ + target: target.name, + filename: "large-file.pdf", + maxFileSize: expect.any(Number), + fileSize: expect.any(Number), + }), + ) + }) + + it("should not emit when file size is within limit", async () => { + const target = + cds.model.definitions["AdminService.Incidents.maximumSizeAttachments"] + + const req = { + target, + data: { + content: Buffer.from("small"), + up__ID: cds.utils.uuid(), + ID: cds.utils.uuid(), + }, + headers: { "content-length": "5" }, + reject: jest.fn(), + } + + const result = await validateAttachmentSize(req) + + expect(result).toBe(true) + expect(req.reject).not.toHaveBeenCalled() + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(attachmentsSvc.emit).not.toHaveBeenCalled() + }) + + it("should still reject when event handler fails", async () => { + attachmentsSvc.emit.mockRejectedValue(new Error("handler error")) + + // Use maximumSizeAttachments which has @Validation.Maximum: '5MB' + const target = + cds.model.definitions["AdminService.Incidents.maximumSizeAttachments"] + + const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } + await INSERT.into(target).entries({ + ...keys, + filename: "large-file.pdf", + status: "Scanning", + }) + + const req = { + target, + data: { + content: { pause: jest.fn() }, + up__ID: keys.up__ID, + ID: keys.ID, + }, + headers: { "content-length": "999999999999" }, + reject: jest.fn(), + } + + await validateAttachmentSize(req) + + expect(req.reject).toHaveBeenCalledWith( + expect.objectContaining({ status: 413 }), + ) + }) +}) + +describe("AttachmentDownloadRejected event", () => { + it("should emit when download is rejected due to non-clean scan status", async () => { + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Infected", + lastScan: new Date().toISOString(), + }) + + const attachmentId = cds.utils.uuid() + const req = { + target, + data: { ID: attachmentId }, + req: { url: "/some/path/content" }, + query: { SELECT: { columns: [] } }, + params: [{ ID: attachmentId }], + reject: jest.fn(), + } + + cds.env.requires.attachments = { scan: true } + + await require("../../lib/generic-handlers").validateAttachment(req) + + expect(req.reject).toHaveBeenCalledWith( + 403, + "UnableToDownloadAttachmentScanStatusNotClean", + ) + + expect(attachmentsSvc.emit).toHaveBeenCalledWith( + "AttachmentDownloadRejected", + expect.objectContaining({ + target: target.name, + keys: { ID: attachmentId }, + status: "Infected", + }), + ) + }) + + it("should not emit when scan status is Clean", async () => { + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Clean", + lastScan: new Date().toISOString(), + }) + + const attachmentId = cds.utils.uuid() + const req = { + target, + data: { ID: attachmentId }, + req: { url: "/some/path/content" }, + query: { SELECT: { columns: [] } }, + params: [{ ID: attachmentId }], + reject: jest.fn(), + } + + cds.env.requires.attachments = { scan: true } + + await require("../../lib/generic-handlers").validateAttachment(req) + + expect(req.reject).not.toHaveBeenCalled() + expect(attachmentsSvc.emit).not.toHaveBeenCalledWith( + "AttachmentDownloadRejected", + expect.anything(), + ) + }) +}) diff --git a/tests/unit/validateAttachmentMimeType.test.js b/tests/unit/validateAttachmentMimeType.test.js index 379ecb65..44c31c12 100644 --- a/tests/unit/validateAttachmentMimeType.test.js +++ b/tests/unit/validateAttachmentMimeType.test.js @@ -241,33 +241,33 @@ describe("validateAttachmentMimeType - Content-Type header bypass security test" }) describe("validateAttachmentMimeType - Unit tests", () => { - it("should return false when target is not an attachments entity", () => { + it("should return false when target is not an attachments entity", async () => { const req = { target: { _attachments: { isAttachmentsEntity: false } }, data: { content: "test" }, reject: jest.fn(), } - const result = validateAttachmentMimeType(req) + const result = await validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).not.toHaveBeenCalled() }) - it("should return false when there is no content", () => { + it("should return false when there is no content", async () => { const req = { target: { _attachments: { isAttachmentsEntity: true } }, data: {}, reject: jest.fn(), } - const result = validateAttachmentMimeType(req) + const result = await validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).not.toHaveBeenCalled() }) - it("should reject when mimeType does not match acceptable media types", () => { + it("should reject when mimeType does not match acceptable media types", async () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -284,7 +284,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = validateAttachmentMimeType(req) + const result = await validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).toHaveBeenCalledWith( @@ -296,7 +296,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { ) }) - it("should return true when mimeType matches acceptable media types", () => { + it("should return true when mimeType matches acceptable media types", async () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -313,13 +313,13 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = validateAttachmentMimeType(req) + const result = await validateAttachmentMimeType(req) expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() }) - it("should allow any mimeType when @Core.AcceptableMediaTypes is not defined (defaults to */*)", () => { + it("should allow any mimeType when @Core.AcceptableMediaTypes is not defined (defaults to */*)", async () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -334,7 +334,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = validateAttachmentMimeType(req) + const result = await validateAttachmentMimeType(req) expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() From c47b7d184260c5fd4378c128827dea1d80dec85d Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 10:06:22 +0100 Subject: [PATCH 02/22] feat: add acceptable media types to AttachmentUploadRejected event --- lib/generic-handlers.js | 1 + tests/unit/rejectionEvents.test.js | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 5549168e..3c96b3a8 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -311,6 +311,7 @@ async function validateAttachmentMimeType(req) { keys: req.data.ID ? { ID: req.data.ID } : {}, filename: req.data.filename, mimeType, + acceptableMediaTypes, reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, }).catch((err) => LOG.error("Failed to emit AttachmentUploadRejected", err)) req.reject(400, "AttachmentMimeTypeDisallowed", { diff --git a/tests/unit/rejectionEvents.test.js b/tests/unit/rejectionEvents.test.js index 94fdd8f8..e5b43ecb 100644 --- a/tests/unit/rejectionEvents.test.js +++ b/tests/unit/rejectionEvents.test.js @@ -71,6 +71,7 @@ describe("AttachmentUploadRejected event", () => { keys: { ID: "att-123" }, filename: "notes.txt", mimeType: "text/plain", + acceptableMediaTypes: ["image/jpeg", "image/png"], reason: expect.stringContaining("@Core.AcceptableMediaTypes"), }), ) From 823a3e598af3f59f0d9c9ec3df1130749f1ee060 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 10:49:56 +0100 Subject: [PATCH 03/22] refactor: await event Co-authored-by: Marten Schiwek --- lib/generic-handlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 3c96b3a8..f6cf49e9 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -151,7 +151,7 @@ async function validateAttachment(req) { if (scanEnabled) { if (status !== "Clean") { - AttachmentsSrv.emit("AttachmentDownloadRejected", { + await AttachmentsSrv.emit("AttachmentDownloadRejected", { target: req.target.name, keys: { ID: attachmentId }, status, From fae766f312b46fbc266612f18fb412df0195d692 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 10:50:03 +0100 Subject: [PATCH 04/22] refactor: await event Co-authored-by: Marten Schiwek --- lib/generic-handlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index f6cf49e9..81751ccb 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -271,7 +271,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { .where({ up__ID: req.data.up__ID }) const AttachmentsSrv = await cds.connect.to("attachments") - AttachmentsSrv.emit("AttachmentSizeExceeded", { + await AttachmentsSrv.emit("AttachmentSizeExceeded", { target: req.target.name, keys: req.data.ID ? { ID: req.data.ID } : {}, filename: attachmentRef?.filename ?? "n/a", From 94e633bffee50440c3ae1694c840e636fa7f484f Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 10:50:12 +0100 Subject: [PATCH 05/22] refactor: await event Co-authored-by: Marten Schiwek --- lib/generic-handlers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 81751ccb..92f30846 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -306,7 +306,7 @@ async function validateAttachmentMimeType(req) { req.target.elements.content["@Core.AcceptableMediaTypes"] || "*/*" if (!checkMimeTypeMatch(acceptableMediaTypes, mimeType)) { const AttachmentsSrv = await cds.connect.to("attachments") - AttachmentsSrv.emit("AttachmentUploadRejected", { + await AttachmentsSrv.emit("AttachmentUploadRejected", { target: req.target.name, keys: req.data.ID ? { ID: req.data.ID } : {}, filename: req.data.filename, From 8f6dbe648827fbefc47a30748c604ee4380f6073 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 15:20:43 +0100 Subject: [PATCH 06/22] fix: emit events inside cds.spawn to get consumers possibility to react even if transaction is rolled back --- lib/generic-handlers.js | 65 ++++++----- tests/unit/rejectionEvents.test.js | 166 +++++++++++++++++++++++++++-- 2 files changed, 196 insertions(+), 35 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 3c96b3a8..3df3c52e 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -151,13 +151,18 @@ async function validateAttachment(req) { if (scanEnabled) { if (status !== "Clean") { - AttachmentsSrv.emit("AttachmentDownloadRejected", { - target: req.target.name, - keys: { ID: attachmentId }, - status, - }).catch((err) => - LOG.error("Failed to emit AttachmentDownloadRejected", err), - ) + cds.spawn(async () => { + try { + const srv = await cds.connect.to("attachments") + await srv.emit("AttachmentDownloadRejected", { + target: req.target.name, + keys: { ID: attachmentId }, + status, + }) + } catch (err) { + LOG.error("Failed to emit AttachmentDownloadRejected", err) + } + }) req.reject(403, "UnableToDownloadAttachmentScanStatusNotClean") } @@ -270,14 +275,20 @@ async function validateAttachmentSize(req, validateContentLength = false) { .from(req.target) .where({ up__ID: req.data.up__ID }) - const AttachmentsSrv = await cds.connect.to("attachments") - AttachmentsSrv.emit("AttachmentSizeExceeded", { - target: req.target.name, - keys: req.data.ID ? { ID: req.data.ID } : {}, - filename: attachmentRef?.filename ?? "n/a", - fileSize: length, - maxFileSize, - }).catch((err) => LOG.error("Failed to emit AttachmentSizeExceeded", err)) + cds.spawn(async () => { + try { + const AttachmentsSrv = await cds.connect.to("attachments") + await AttachmentsSrv.emit("AttachmentSizeExceeded", { + target: req.target.name, + keys: req.data.ID ? { ID: req.data.ID } : {}, + filename: attachmentRef?.filename ?? "n/a", + fileSize: length, + maxFileSize, + }) + } catch (err) { + LOG.error("Failed to emit AttachmentSizeExceeded", err) + } + }) req.reject({ status: 413, @@ -305,15 +316,21 @@ async function validateAttachmentMimeType(req) { const acceptableMediaTypes = req.target.elements.content["@Core.AcceptableMediaTypes"] || "*/*" if (!checkMimeTypeMatch(acceptableMediaTypes, mimeType)) { - const AttachmentsSrv = await cds.connect.to("attachments") - AttachmentsSrv.emit("AttachmentUploadRejected", { - target: req.target.name, - keys: req.data.ID ? { ID: req.data.ID } : {}, - filename: req.data.filename, - mimeType, - acceptableMediaTypes, - reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, - }).catch((err) => LOG.error("Failed to emit AttachmentUploadRejected", err)) + cds.spawn(async () => { + try { + const AttachmentsSrv = await cds.connect.to("attachments") + await AttachmentsSrv.emit("AttachmentUploadRejected", { + target: req.target.name, + keys: req.data.ID ? { ID: req.data.ID } : {}, + filename: req.data.filename, + mimeType, + acceptableMediaTypes, + reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, + }) + } catch (err) { + LOG.error("Failed to emit AttachmentUploadRejected", err) + } + }) req.reject(400, "AttachmentMimeTypeDisallowed", { mimeType: mimeType, }) diff --git a/tests/unit/rejectionEvents.test.js b/tests/unit/rejectionEvents.test.js index e5b43ecb..e3ab8ae7 100644 --- a/tests/unit/rejectionEvents.test.js +++ b/tests/unit/rejectionEvents.test.js @@ -11,6 +11,8 @@ const { let attachmentsSvc let originalConnectTo +let spawnCallback +let originalSpawn beforeEach(() => { jest.restoreAllMocks() @@ -24,10 +26,18 @@ beforeEach(() => { if (name === "attachments") return Promise.resolve(attachmentsSvc) return originalConnectTo.call(cds.connect, name) }) + + spawnCallback = null + originalSpawn = cds.spawn + cds.spawn = jest.fn().mockImplementation((fn) => { + spawnCallback = fn + return { on: jest.fn() } + }) }) afterEach(() => { cds.connect.to = originalConnectTo + cds.spawn = originalSpawn }) describe("AttachmentUploadRejected event", () => { @@ -61,8 +71,8 @@ describe("AttachmentUploadRejected event", () => { }, ) - // Wait for the fire-and-forget promise to resolve - await new Promise((resolve) => setTimeout(resolve, 0)) + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + await spawnCallback() expect(attachmentsSvc.emit).toHaveBeenCalledWith( "AttachmentUploadRejected", @@ -101,9 +111,7 @@ describe("AttachmentUploadRejected event", () => { expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() - - await new Promise((resolve) => setTimeout(resolve, 0)) - expect(attachmentsSvc.emit).not.toHaveBeenCalled() + expect(cds.spawn).not.toHaveBeenCalled() }) it("should still reject when event handler fails", async () => { @@ -138,6 +146,44 @@ describe("AttachmentUploadRejected event", () => { mimeType: "text/plain", }, ) + + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + // spawn callback should not throw even though emit rejects + await expect(spawnCallback()).resolves.toBeUndefined() + }) + + it("should emit via cds.spawn to survive req.reject transaction rollback", async () => { + const req = { + target: { + name: "TestService.Attachments", + _attachments: { isAttachmentsEntity: true }, + elements: { + content: { + "@Core.AcceptableMediaTypes": ["image/jpeg"], + }, + }, + }, + data: { + content: "test content", + mimeType: "text/plain", + ID: "att-123", + filename: "notes.txt", + }, + reject: jest.fn(), + } + + await validateAttachmentMimeType(req) + + // cds.spawn is called, proving the emit runs in a separate transaction + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + + // Even after req.reject was called, the spawn callback can still emit + expect(req.reject).toHaveBeenCalled() + await spawnCallback() + expect(attachmentsSvc.emit).toHaveBeenCalledWith( + "AttachmentUploadRejected", + expect.anything(), + ) }) }) @@ -174,8 +220,8 @@ describe("AttachmentSizeExceeded event", () => { }), ) - // Wait for fire-and-forget promise - await new Promise((resolve) => setTimeout(resolve, 0)) + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + await spawnCallback() expect(attachmentsSvc.emit).toHaveBeenCalledWith( "AttachmentSizeExceeded", @@ -207,9 +253,7 @@ describe("AttachmentSizeExceeded event", () => { expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() - - await new Promise((resolve) => setTimeout(resolve, 0)) - expect(attachmentsSvc.emit).not.toHaveBeenCalled() + expect(cds.spawn).not.toHaveBeenCalled() }) it("should still reject when event handler fails", async () => { @@ -242,6 +286,42 @@ describe("AttachmentSizeExceeded event", () => { expect(req.reject).toHaveBeenCalledWith( expect.objectContaining({ status: 413 }), ) + + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + await expect(spawnCallback()).resolves.toBeUndefined() + }) + + it("should emit via cds.spawn to survive req.reject transaction rollback", async () => { + const target = + cds.model.definitions["AdminService.Incidents.maximumSizeAttachments"] + + const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } + await INSERT.into(target).entries({ + ...keys, + filename: "large-file.pdf", + status: "Scanning", + }) + + const req = { + target, + data: { + content: { pause: jest.fn() }, + up__ID: keys.up__ID, + ID: keys.ID, + }, + headers: { "content-length": "999999999999" }, + reject: jest.fn(), + } + + await validateAttachmentSize(req) + + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + expect(req.reject).toHaveBeenCalled() + await spawnCallback() + expect(attachmentsSvc.emit).toHaveBeenCalledWith( + "AttachmentSizeExceeded", + expect.anything(), + ) }) }) @@ -273,6 +353,9 @@ describe("AttachmentDownloadRejected event", () => { "UnableToDownloadAttachmentScanStatusNotClean", ) + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + await spawnCallback() + expect(attachmentsSvc.emit).toHaveBeenCalledWith( "AttachmentDownloadRejected", expect.objectContaining({ @@ -306,7 +389,68 @@ describe("AttachmentDownloadRejected event", () => { await require("../../lib/generic-handlers").validateAttachment(req) expect(req.reject).not.toHaveBeenCalled() - expect(attachmentsSvc.emit).not.toHaveBeenCalledWith( + expect(cds.spawn).not.toHaveBeenCalled() + }) + + it("should still reject when event handler fails", async () => { + attachmentsSvc.emit.mockRejectedValue(new Error("handler error")) + + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Infected", + lastScan: new Date().toISOString(), + }) + + const attachmentId = cds.utils.uuid() + const req = { + target, + data: { ID: attachmentId }, + req: { url: "/some/path/content" }, + query: { SELECT: { columns: [] } }, + params: [{ ID: attachmentId }], + reject: jest.fn(), + } + + cds.env.requires.attachments = { scan: true } + + await require("../../lib/generic-handlers").validateAttachment(req) + + expect(req.reject).toHaveBeenCalledWith( + 403, + "UnableToDownloadAttachmentScanStatusNotClean", + ) + + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + await expect(spawnCallback()).resolves.toBeUndefined() + }) + + it("should emit via cds.spawn to survive req.reject transaction rollback", async () => { + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Infected", + lastScan: new Date().toISOString(), + }) + + const attachmentId = cds.utils.uuid() + const req = { + target, + data: { ID: attachmentId }, + req: { url: "/some/path/content" }, + query: { SELECT: { columns: [] } }, + params: [{ ID: attachmentId }], + reject: jest.fn(), + } + + cds.env.requires.attachments = { scan: true } + + await require("../../lib/generic-handlers").validateAttachment(req) + + expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) + expect(req.reject).toHaveBeenCalled() + await spawnCallback() + expect(attachmentsSvc.emit).toHaveBeenCalledWith( "AttachmentDownloadRejected", expect.anything(), ) From e8f387281dcd2ec36b7284ce1d7b786b9a78d745 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 13 Mar 2026 15:24:17 +0100 Subject: [PATCH 07/22] fix: linting --- lib/generic-handlers.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index d675f509..3df3c52e 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -241,9 +241,9 @@ async function validateAttachmentSize(req, validateContentLength = false) { const maxFileSize = req.target.elements["content"]["@Validation.Maximum"] ? (sizeInBytes( - req.target.elements["content"]["@Validation.Maximum"], - req.target.name, - ) ?? MAX_FILE_SIZE) + req.target.elements["content"]["@Validation.Maximum"], + req.target.name, + ) ?? MAX_FILE_SIZE) : MAX_FILE_SIZE if (!validateContentLength) { From 0dcbfd66b400a25a28b6d48b3ee872def1df787d Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 10:22:56 +0100 Subject: [PATCH 08/22] Adding audit-logging for sec events --- README.md | 16 ++++++++++++++++ lib/generic-handlers.js | 3 +++ lib/helper.js | 10 ++++++++++ srv/basic.js | 16 ++++++++++++++++ tests/incidents-app/package.json | 1 + 5 files changed, 46 insertions(+) diff --git a/README.md b/README.md index 6d1732fc..8c333f00 100755 --- a/README.md +++ b/README.md @@ -242,6 +242,22 @@ According to the recommendation of the [Malware Scanning Service](http://help.sa By default, `scanExpiryMs` is set to `259200000` milliseconds (3 days). Downloading an attachment is not permitted unless its status is `Clean`. +### Audit logging + +The attachment service emits the following three events: +- AttachmentDownloadRejected, +- AttachmentSizeExceeded, +- AttachmentUploadRejected + +When `@cap-js/audit-logging` is a dependency of your app, the three events will be automatically logged as security events in the audit log service. + +You can register custom handlers for the three events by writing: + +```js +const attachments = await cds.connect.to('attachments'); +attachments.on('AttachmentDownloadRejected', msg => {}) +``` + ### Visibility Control for Attachments UI Facet Generation By setting the `@UI.Hidden` property to `true`, developers can hide the visibility of the plugin in the UI. This feature is particularly useful in scenarios where the visibility of the plugin needs to be dynamically controlled based on certain conditions. diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 3df3c52e..1ff16cd4 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -158,6 +158,7 @@ async function validateAttachment(req) { target: req.target.name, keys: { ID: attachmentId }, status, + ipAddress: req.req?.socket?.remoteAddress }) } catch (err) { LOG.error("Failed to emit AttachmentDownloadRejected", err) @@ -284,6 +285,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { filename: attachmentRef?.filename ?? "n/a", fileSize: length, maxFileSize, + ipAddress: req.req?.socket?.remoteAddress }) } catch (err) { LOG.error("Failed to emit AttachmentSizeExceeded", err) @@ -326,6 +328,7 @@ async function validateAttachmentMimeType(req) { mimeType, acceptableMediaTypes, reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, + ipAddress: req.req?.socket?.remoteAddress }) } catch (err) { LOG.error("Failed to emit AttachmentUploadRejected", err) diff --git a/lib/helper.js b/lib/helper.js index d399dc25..e526b2f6 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -611,6 +611,15 @@ function createSizeCheckHandler({ return { handler, getSizeExceeded, createError } } +function hasAuditLogging() { + try { + require('@cap-js/audit-logging') + return true + } catch { + return false + } +} + module.exports = { fetchToken, getObjectStoreCredentials, @@ -625,4 +634,5 @@ module.exports = { inferTargetCAP8, getAttachmentKind, createSizeCheckHandler, + hasAuditLogging } diff --git a/srv/basic.js b/srv/basic.js index 4ab063dc..99f6d3d6 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -4,6 +4,7 @@ const { computeHash, traverseEntity, buildBackAssocChain, + hasAuditLogging, } = require("../lib/helper") class AttachmentsService extends cds.Service { @@ -46,6 +47,21 @@ class AttachmentsService extends cds.Service { ) } }) + + if (hasAuditLogging()) { + this.on(['AttachmentDownloadRejected', 'AttachmentSizeExceeded', 'AttachmentUploadRejected'], async msg => { + const audit = await cds.connect.to('audit-log') + const ipAddress = msg.data.ipAddress; + if (ipAddress) delete msg.data.ipAddress; + await audit.log('SecurityEvent', { + data: Object.assign({ + event: msg.event + }, msg.data), + ip: ipAddress ?? undefined + }) + }) + } + return super.init() } diff --git a/tests/incidents-app/package.json b/tests/incidents-app/package.json index 98df5c54..4d85beaf 100644 --- a/tests/incidents-app/package.json +++ b/tests/incidents-app/package.json @@ -3,6 +3,7 @@ "version": "1.0.0", "dependencies": { "@cap-js/attachments": "file:../../.", + "@cap-js/audit-logging": "^1.2.0", "@cap-js/hana": "2.6.0" }, "cds": { From d50c7be526358b0d884a9316f8a983bc409c5789 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 10:23:10 +0100 Subject: [PATCH 09/22] Formatting --- README.md | 7 ++++--- lib/generic-handlers.js | 6 +++--- lib/helper.js | 4 ++-- srv/basic.js | 32 +++++++++++++++++++++----------- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 8c333f00..00a630c7 100755 --- a/README.md +++ b/README.md @@ -245,7 +245,8 @@ By default, `scanExpiryMs` is set to `259200000` milliseconds (3 days). Download ### Audit logging The attachment service emits the following three events: -- AttachmentDownloadRejected, + +- AttachmentDownloadRejected, - AttachmentSizeExceeded, - AttachmentUploadRejected @@ -254,8 +255,8 @@ When `@cap-js/audit-logging` is a dependency of your app, the three events will You can register custom handlers for the three events by writing: ```js -const attachments = await cds.connect.to('attachments'); -attachments.on('AttachmentDownloadRejected', msg => {}) +const attachments = await cds.connect.to("attachments") +attachments.on("AttachmentDownloadRejected", (msg) => {}) ``` ### Visibility Control for Attachments UI Facet Generation diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 1ff16cd4..41d53b47 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -158,7 +158,7 @@ async function validateAttachment(req) { target: req.target.name, keys: { ID: attachmentId }, status, - ipAddress: req.req?.socket?.remoteAddress + ipAddress: req.req?.socket?.remoteAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentDownloadRejected", err) @@ -285,7 +285,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { filename: attachmentRef?.filename ?? "n/a", fileSize: length, maxFileSize, - ipAddress: req.req?.socket?.remoteAddress + ipAddress: req.req?.socket?.remoteAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentSizeExceeded", err) @@ -328,7 +328,7 @@ async function validateAttachmentMimeType(req) { mimeType, acceptableMediaTypes, reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, - ipAddress: req.req?.socket?.remoteAddress + ipAddress: req.req?.socket?.remoteAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentUploadRejected", err) diff --git a/lib/helper.js b/lib/helper.js index e526b2f6..09dfa156 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -613,7 +613,7 @@ function createSizeCheckHandler({ function hasAuditLogging() { try { - require('@cap-js/audit-logging') + require("@cap-js/audit-logging") return true } catch { return false @@ -634,5 +634,5 @@ module.exports = { inferTargetCAP8, getAttachmentKind, createSizeCheckHandler, - hasAuditLogging + hasAuditLogging, } diff --git a/srv/basic.js b/srv/basic.js index 99f6d3d6..447d5d2c 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -49,17 +49,27 @@ class AttachmentsService extends cds.Service { }) if (hasAuditLogging()) { - this.on(['AttachmentDownloadRejected', 'AttachmentSizeExceeded', 'AttachmentUploadRejected'], async msg => { - const audit = await cds.connect.to('audit-log') - const ipAddress = msg.data.ipAddress; - if (ipAddress) delete msg.data.ipAddress; - await audit.log('SecurityEvent', { - data: Object.assign({ - event: msg.event - }, msg.data), - ip: ipAddress ?? undefined - }) - }) + this.on( + [ + "AttachmentDownloadRejected", + "AttachmentSizeExceeded", + "AttachmentUploadRejected", + ], + async (msg) => { + const audit = await cds.connect.to("audit-log") + const ipAddress = msg.data.ipAddress + if (ipAddress) delete msg.data.ipAddress + await audit.log("SecurityEvent", { + data: Object.assign( + { + event: msg.event, + }, + msg.data, + ), + ip: ipAddress ?? undefined, + }) + }, + ) } return super.init() From b3095ea63eeee584100937c84f2735b633398e4e Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 10:26:23 +0100 Subject: [PATCH 10/22] Update basic.js --- srv/basic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srv/basic.js b/srv/basic.js index 447d5d2c..eab84395 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -66,7 +66,7 @@ class AttachmentsService extends cds.Service { }, msg.data, ), - ip: ipAddress ?? undefined, + ip: ipAddress || undefined, }) }, ) From 9e6f796961fdf47f0ac23a858b96699af548336c Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 10:27:58 +0100 Subject: [PATCH 11/22] Update basic.js --- srv/basic.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/srv/basic.js b/srv/basic.js index eab84395..a9ffecbb 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -1,5 +1,6 @@ const cds = require("@sap/cds") const LOG = cds.log("attachments") +const DEBUG = cds.debug("attachments") const { computeHash, traverseEntity, @@ -49,6 +50,7 @@ class AttachmentsService extends cds.Service { }) if (hasAuditLogging()) { + DEBUG && DEBUG(`Register audit logging handlers for security events.`) this.on( [ "AttachmentDownloadRejected", From f7a02c104f71e1b5c17600165068ea257b6660b4 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 12:39:58 +0100 Subject: [PATCH 12/22] Create auditLogging.test.js --- tests/unit/auditLogging.test.js | 117 ++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 tests/unit/auditLogging.test.js diff --git a/tests/unit/auditLogging.test.js b/tests/unit/auditLogging.test.js new file mode 100644 index 00000000..83d21892 --- /dev/null +++ b/tests/unit/auditLogging.test.js @@ -0,0 +1,117 @@ +require("../../lib/csn-runtime-extension") +const cds = require("@sap/cds") +cds.env.requires["audit-log"] = { + impl: "@cap-js/audit-logging/srv/log2console", + outbox: false, +} +const { join } = cds.utils.path +const app = join(__dirname, "../incidents-app") +cds.test(app) + +let attachmentsSvc + +beforeEach(async () => { + const svc = await cds.connect.to("attachments") + attachmentsSvc = cds.unboxed(svc) +}) + +describe("Audit logging for security events (audit-logging dependency present)", () => { + const log = cds.test.log() + + it("should log AttachmentDownloadRejected as SecurityEvent", async () => { + await attachmentsSvc.emit("AttachmentDownloadRejected", { + target: "AdminService.Incidents.attachments", + keys: { ID: "att-001" }, + status: "Infected", + ipAddress: "10.0.0.1", + }) + + expect(log.output).toContain("[audit-log] - SecurityEvent:") + expect(log.output).toContain("AttachmentDownloadRejected") + expect(log.output).toContain("Infected") + }) + + it("should log AttachmentSizeExceeded as SecurityEvent", async () => { + await attachmentsSvc.emit("AttachmentSizeExceeded", { + target: "AdminService.Incidents.attachments", + keys: { ID: "att-002" }, + filename: "large-file.pdf", + fileSize: 999999999, + maxFileSize: 5242880, + ipAddress: "192.168.1.10", + }) + + expect(log.output).toContain("[audit-log] - SecurityEvent:") + expect(log.output).toContain("AttachmentSizeExceeded") + expect(log.output).toContain("large-file.pdf") + expect(log.output).toContain("999999999") + expect(log.output).toContain("5242880") + }) + + it("should log AttachmentUploadRejected as SecurityEvent", async () => { + await attachmentsSvc.emit("AttachmentUploadRejected", { + target: "AdminService.Incidents.attachments", + keys: { ID: "att-003" }, + filename: "script.exe", + mimeType: "application/x-msdownload", + acceptableMediaTypes: ["image/jpeg", "image/png"], + reason: + "MIME type 'application/x-msdownload' is not in @Core.AcceptableMediaTypes", + ipAddress: "172.16.0.5", + }) + + expect(log.output).toContain("[audit-log] - SecurityEvent:") + expect(log.output).toContain("AttachmentUploadRejected") + expect(log.output).toContain("script.exe") + expect(log.output).toContain("application/x-msdownload") + }) +}) + +describe("Audit logging when audit-logging dependency is absent", () => { + const log = cds.test.log() + let helperModule + let originalHasAuditLogging + + beforeAll(() => { + helperModule = require("../../lib/helper") + originalHasAuditLogging = helperModule.hasAuditLogging + }) + + afterAll(() => { + helperModule.hasAuditLogging = originalHasAuditLogging + }) + + it("should not register audit log handlers when hasAuditLogging returns false", async () => { + // Override hasAuditLogging to return false + helperModule.hasAuditLogging = () => false + + // Create a fresh AttachmentsService instance with audit logging disabled + const AttachmentsService = require("../../srv/basic") + const svc = new AttachmentsService() + svc.model = cds.model + // Stub super.init() to avoid full service bootstrap + const origInit = Object.getPrototypeOf(AttachmentsService.prototype).init + Object.getPrototypeOf(AttachmentsService.prototype).init = jest + .fn() + .mockResolvedValue(undefined) + + await svc.init() + + // Restore super.init + Object.getPrototypeOf(AttachmentsService.prototype).init = origInit + + // The service should have handlers for DeleteAttachment and DeleteInfectedAttachment + // but NOT for the security events routed to audit logging + const registeredEvents = (svc._handlers?.on || []).map((h) => + Array.isArray(h.for) ? h.for : [h.for], + ) + const flatEvents = registeredEvents.flat().filter(Boolean) + + expect(flatEvents).not.toContain("AttachmentDownloadRejected") + expect(flatEvents).not.toContain("AttachmentSizeExceeded") + expect(flatEvents).not.toContain("AttachmentUploadRejected") + + // Verify no audit log output was produced + expect(log.output).not.toContain("[audit-log] - SecurityEvent:") + }) +}) From 8f918add6dd801cd15d3cf51a0e622e83cccb958 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 13:02:22 +0100 Subject: [PATCH 13/22] Cleanup --- lib/generic-handlers.js | 10 ++++++---- lib/helper.js | 10 ---------- srv/basic.js | 13 +++---------- tests/unit/auditLogging.test.js | 17 ++++------------- 4 files changed, 13 insertions(+), 37 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 41d53b47..bb046496 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -151,6 +151,7 @@ async function validateAttachment(req) { if (scanEnabled) { if (status !== "Clean") { + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const srv = await cds.connect.to("attachments") @@ -158,7 +159,7 @@ async function validateAttachment(req) { target: req.target.name, keys: { ID: attachmentId }, status, - ipAddress: req.req?.socket?.remoteAddress, + ipAddress }) } catch (err) { LOG.error("Failed to emit AttachmentDownloadRejected", err) @@ -275,7 +276,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { const attachmentRef = await SELECT.one("filename") .from(req.target) .where({ up__ID: req.data.up__ID }) - + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const AttachmentsSrv = await cds.connect.to("attachments") @@ -285,7 +286,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { filename: attachmentRef?.filename ?? "n/a", fileSize: length, maxFileSize, - ipAddress: req.req?.socket?.remoteAddress, + ipAddress }) } catch (err) { LOG.error("Failed to emit AttachmentSizeExceeded", err) @@ -318,6 +319,7 @@ async function validateAttachmentMimeType(req) { const acceptableMediaTypes = req.target.elements.content["@Core.AcceptableMediaTypes"] || "*/*" if (!checkMimeTypeMatch(acceptableMediaTypes, mimeType)) { + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const AttachmentsSrv = await cds.connect.to("attachments") @@ -328,7 +330,7 @@ async function validateAttachmentMimeType(req) { mimeType, acceptableMediaTypes, reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, - ipAddress: req.req?.socket?.remoteAddress, + ipAddress }) } catch (err) { LOG.error("Failed to emit AttachmentUploadRejected", err) diff --git a/lib/helper.js b/lib/helper.js index 09dfa156..d399dc25 100644 --- a/lib/helper.js +++ b/lib/helper.js @@ -611,15 +611,6 @@ function createSizeCheckHandler({ return { handler, getSizeExceeded, createError } } -function hasAuditLogging() { - try { - require("@cap-js/audit-logging") - return true - } catch { - return false - } -} - module.exports = { fetchToken, getObjectStoreCredentials, @@ -634,5 +625,4 @@ module.exports = { inferTargetCAP8, getAttachmentKind, createSizeCheckHandler, - hasAuditLogging, } diff --git a/srv/basic.js b/srv/basic.js index a9ffecbb..1cddddfd 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -5,7 +5,6 @@ const { computeHash, traverseEntity, buildBackAssocChain, - hasAuditLogging, } = require("../lib/helper") class AttachmentsService extends cds.Service { @@ -49,7 +48,7 @@ class AttachmentsService extends cds.Service { } }) - if (hasAuditLogging()) { + if (cds.env.requires["audit-log"]) { DEBUG && DEBUG(`Register audit logging handlers for security events.`) this.on( [ @@ -59,15 +58,9 @@ class AttachmentsService extends cds.Service { ], async (msg) => { const audit = await cds.connect.to("audit-log") - const ipAddress = msg.data.ipAddress - if (ipAddress) delete msg.data.ipAddress + const { ipAddress, ...eventData } = msg.data await audit.log("SecurityEvent", { - data: Object.assign( - { - event: msg.event, - }, - msg.data, - ), + data: { event: msg.event, ...eventData }, ip: ipAddress || undefined, }) }, diff --git a/tests/unit/auditLogging.test.js b/tests/unit/auditLogging.test.js index 83d21892..c2d55673 100644 --- a/tests/unit/auditLogging.test.js +++ b/tests/unit/auditLogging.test.js @@ -67,23 +67,13 @@ describe("Audit logging for security events (audit-logging dependency present)", }) }) -describe("Audit logging when audit-logging dependency is absent", () => { +describe("Audit logging when audit-logging is disabled", () => { const log = cds.test.log() - let helperModule - let originalHasAuditLogging - - beforeAll(() => { - helperModule = require("../../lib/helper") - originalHasAuditLogging = helperModule.hasAuditLogging - }) - - afterAll(() => { - helperModule.hasAuditLogging = originalHasAuditLogging - }) it("should not register audit log handlers when hasAuditLogging returns false", async () => { // Override hasAuditLogging to return false - helperModule.hasAuditLogging = () => false + const originalLog = cds.env.requires['audit-log'] + cds.env.requires['audit-log'] = false // Create a fresh AttachmentsService instance with audit logging disabled const AttachmentsService = require("../../srv/basic") @@ -113,5 +103,6 @@ describe("Audit logging when audit-logging dependency is absent", () => { // Verify no audit log output was produced expect(log.output).not.toContain("[audit-log] - SecurityEvent:") + cds.env.requires['audit-log'] = originalLog; }) }) From 38b48d0cb9573a36a83d25bd5035cddac6431c1c Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 13:02:30 +0100 Subject: [PATCH 14/22] Formatting --- lib/generic-handlers.js | 12 ++++++------ tests/unit/auditLogging.test.js | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index bb046496..e4a89974 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -151,7 +151,7 @@ async function validateAttachment(req) { if (scanEnabled) { if (status !== "Clean") { - const ipAddress = req.req?.socket?.remoteAddress + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const srv = await cds.connect.to("attachments") @@ -159,7 +159,7 @@ async function validateAttachment(req) { target: req.target.name, keys: { ID: attachmentId }, status, - ipAddress + ipAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentDownloadRejected", err) @@ -276,7 +276,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { const attachmentRef = await SELECT.one("filename") .from(req.target) .where({ up__ID: req.data.up__ID }) - const ipAddress = req.req?.socket?.remoteAddress + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const AttachmentsSrv = await cds.connect.to("attachments") @@ -286,7 +286,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { filename: attachmentRef?.filename ?? "n/a", fileSize: length, maxFileSize, - ipAddress + ipAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentSizeExceeded", err) @@ -319,7 +319,7 @@ async function validateAttachmentMimeType(req) { const acceptableMediaTypes = req.target.elements.content["@Core.AcceptableMediaTypes"] || "*/*" if (!checkMimeTypeMatch(acceptableMediaTypes, mimeType)) { - const ipAddress = req.req?.socket?.remoteAddress + const ipAddress = req.req?.socket?.remoteAddress cds.spawn(async () => { try { const AttachmentsSrv = await cds.connect.to("attachments") @@ -330,7 +330,7 @@ async function validateAttachmentMimeType(req) { mimeType, acceptableMediaTypes, reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`, - ipAddress + ipAddress, }) } catch (err) { LOG.error("Failed to emit AttachmentUploadRejected", err) diff --git a/tests/unit/auditLogging.test.js b/tests/unit/auditLogging.test.js index c2d55673..be223bd1 100644 --- a/tests/unit/auditLogging.test.js +++ b/tests/unit/auditLogging.test.js @@ -72,8 +72,8 @@ describe("Audit logging when audit-logging is disabled", () => { it("should not register audit log handlers when hasAuditLogging returns false", async () => { // Override hasAuditLogging to return false - const originalLog = cds.env.requires['audit-log'] - cds.env.requires['audit-log'] = false + const originalLog = cds.env.requires["audit-log"] + cds.env.requires["audit-log"] = false // Create a fresh AttachmentsService instance with audit logging disabled const AttachmentsService = require("../../srv/basic") @@ -103,6 +103,6 @@ describe("Audit logging when audit-logging is disabled", () => { // Verify no audit log output was produced expect(log.output).not.toContain("[audit-log] - SecurityEvent:") - cds.env.requires['audit-log'] = originalLog; + cds.env.requires["audit-log"] = originalLog }) }) From f1f104b7aae3bf6f6153f30b4772d72b6dbecc24 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 14:30:23 +0100 Subject: [PATCH 15/22] Update testUtils.js --- tests/utils/testUtils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 3cf38f5a..b643b6e8 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -72,7 +72,7 @@ async function waitForDeletion(attachmentID) { } AttachmentsSrv.on("DeleteAttachment", handler) }), - delay(30000).then(() => { + delay(35000).then(() => { throw new Error( `Timeout waiting for deletion of attachment ID: ${attachmentID}`, ) @@ -103,7 +103,7 @@ async function waitForMalwareDeletion(attachmentID) { } AttachmentsSrv.on("DeleteInfectedAttachment", handler) }), - delay(30000).then(() => { + delay(35000).then(() => { throw new Error( `Timeout waiting for malware deletion of attachment ID: ${attachmentID}`, ) From a9b055c93942fad172888146abec49b6eb52f087 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 15:55:16 +0100 Subject: [PATCH 16/22] Increase outbox processing power --- srv/azure-blob-storage.js | 13 +++++++++++-- srv/basic.js | 6 +++--- srv/gcp.js | 13 +++++++++++-- tests/incidents-app/package.json | 4 ++++ tests/utils/testUtils.js | 4 ++-- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/srv/azure-blob-storage.js b/srv/azure-blob-storage.js index 11e5128d..253fa141 100644 --- a/srv/azure-blob-storage.js +++ b/srv/azure-blob-storage.js @@ -317,9 +317,18 @@ module.exports = class AzureAttachmentsService extends ( ) const blobClient = containerClient.getBlockBlobClient(blobName) - const response = await blobClient.delete() + let response + try { + response = await blobClient.delete() + } catch (error) { + if (error.statusCode === 404) { + response = error + } else { + throw error; + } + } - if (response._response.status !== 202) { + if (response._response?.status !== 202) { LOG.warn("File has not been deleted from Azure Blob Storage", { blobName, containerName: containerClient.containerName, diff --git a/srv/basic.js b/srv/basic.js index 1cddddfd..0370b7d4 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -297,12 +297,12 @@ class AttachmentsService extends cds.Service { for (const attachment of req.attachmentsToDelete) { if (attachment.url) { const attachmentsSrv = await cds.connect.to("attachments") - LOG.info( + LOG.debug( "[deleteAttachmentsWithKeys] Emitting DeleteAttachment for:", attachment.url, ) await attachmentsSrv.emit("DeleteAttachment", attachment) - LOG.info( + LOG.debug( "[deleteAttachmentsWithKeys] Emitted DeleteAttachment for:", attachment.url, ) @@ -313,7 +313,7 @@ class AttachmentsService extends cds.Service { ) } } - LOG.info("[deleteAttachmentsWithKeys] Finished") + LOG.debug("[deleteAttachmentsWithKeys] Finished") } /** diff --git a/srv/gcp.js b/srv/gcp.js index 209a791c..6f0718a7 100644 --- a/srv/gcp.js +++ b/srv/gcp.js @@ -348,8 +348,17 @@ module.exports = class GoogleAttachmentsService extends ( ) const file = bucket.file(blobName) - const response = await file.delete() - if (response[0]?.statusCode !== 204) { + let response + try { + response = await file.delete() + } catch (error) { + if (error.statusCode === 404) { + response = error + } else { + throw error; + } + } + if (response?.[0]?.statusCode !== 204) { LOG.warn("File has not been deleted from Google Cloud Storage", { blobName, bucketName: bucket.name, diff --git a/tests/incidents-app/package.json b/tests/incidents-app/package.json index 4d85beaf..882b1939 100644 --- a/tests/incidents-app/package.json +++ b/tests/incidents-app/package.json @@ -13,6 +13,10 @@ } }, "requires": { + "queue": { + "parallel": true, + "chunkSize": 20 + }, "attachments": { "scanExpiryMs": 30000 }, diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index b643b6e8..3cf38f5a 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -72,7 +72,7 @@ async function waitForDeletion(attachmentID) { } AttachmentsSrv.on("DeleteAttachment", handler) }), - delay(35000).then(() => { + delay(30000).then(() => { throw new Error( `Timeout waiting for deletion of attachment ID: ${attachmentID}`, ) @@ -103,7 +103,7 @@ async function waitForMalwareDeletion(attachmentID) { } AttachmentsSrv.on("DeleteInfectedAttachment", handler) }), - delay(35000).then(() => { + delay(30000).then(() => { throw new Error( `Timeout waiting for malware deletion of attachment ID: ${attachmentID}`, ) From 01645d8ee20c307dd7438c7ad820d2dbfd5cc589 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 15:55:36 +0100 Subject: [PATCH 17/22] Formatting --- srv/azure-blob-storage.js | 2 +- srv/gcp.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/srv/azure-blob-storage.js b/srv/azure-blob-storage.js index 253fa141..040fd26f 100644 --- a/srv/azure-blob-storage.js +++ b/srv/azure-blob-storage.js @@ -324,7 +324,7 @@ module.exports = class AzureAttachmentsService extends ( if (error.statusCode === 404) { response = error } else { - throw error; + throw error } } diff --git a/srv/gcp.js b/srv/gcp.js index 6f0718a7..45477782 100644 --- a/srv/gcp.js +++ b/srv/gcp.js @@ -350,12 +350,12 @@ module.exports = class GoogleAttachmentsService extends ( const file = bucket.file(blobName) let response try { - response = await file.delete() + response = await file.delete() } catch (error) { if (error.statusCode === 404) { response = error } else { - throw error; + throw error } } if (response?.[0]?.statusCode !== 204) { From 996cb8a70e8997725bee5c33ab8461935caf942e Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 25 Mar 2026 08:57:12 +0100 Subject: [PATCH 18/22] Fix merge change --- lib/generic-handlers.js | 4 ++-- lib/plugin.js | 2 +- tests/unit/rejectionEvents.test.js | 8 ++++---- tests/unit/validateAttachmentMimeType.test.js | 20 +++++++++---------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 3f441617..ca279846 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -325,7 +325,7 @@ async function validateAttachmentSize(req, validateContentLength = false) { * Validates the attachment mime type against acceptable media types * @param {import('@sap/cds').Request} req - The request object */ -async function validateAttachmentMimeType(req) { +function validateAttachmentMimeType(req) { if (!req.target?._attachments.isAttachmentsEntity || !req.data.content) return false @@ -362,7 +362,7 @@ async function validateAttachmentMimeType(req) { async function validateAndInsertAttachmentFromDBHandler(data, target, req) { if (!data.content) return - if (!(await validateAttachmentMimeType({ data, target, reject: req.reject }))) + if (!(validateAttachmentMimeType({ data, target, reject: req.reject }))) return if ( !(await validateAttachmentSize({ data, target, reject: req.reject }, true)) diff --git a/lib/plugin.js b/lib/plugin.js index c7baf28f..337a4591 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -188,7 +188,7 @@ cds.ApplicationService.handle_attachments = cds.service.impl(async function () { } } // Validate mimeType against @Core.AcceptableMediaTypes before uploading content - if (!(await validateAttachmentMimeType(req))) return + if (!(validateAttachmentMimeType(req))) return const AttachmentsSrv = await cds.connect.to("attachments") try { return await AttachmentsSrv.put(req.target, req.data) diff --git a/tests/unit/rejectionEvents.test.js b/tests/unit/rejectionEvents.test.js index e3348dcb..0da5b435 100644 --- a/tests/unit/rejectionEvents.test.js +++ b/tests/unit/rejectionEvents.test.js @@ -87,7 +87,7 @@ describe("AttachmentUploadRejected event", () => { ) }) - it("should not emit when MIME type is allowed", async () => { + it("should not emit when MIME type is allowed", () => { const req = { target: { name: "TestService.Attachments", @@ -107,7 +107,7 @@ describe("AttachmentUploadRejected event", () => { reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() @@ -136,7 +136,7 @@ describe("AttachmentUploadRejected event", () => { reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).toHaveBeenCalledWith( @@ -172,7 +172,7 @@ describe("AttachmentUploadRejected event", () => { reject: jest.fn(), } - await validateAttachmentMimeType(req) + validateAttachmentMimeType(req) // cds.spawn is called, proving the emit runs in a separate transaction expect(cds.spawn).toHaveBeenCalledWith(expect.any(Function)) diff --git a/tests/unit/validateAttachmentMimeType.test.js b/tests/unit/validateAttachmentMimeType.test.js index 44c31c12..379ecb65 100644 --- a/tests/unit/validateAttachmentMimeType.test.js +++ b/tests/unit/validateAttachmentMimeType.test.js @@ -241,33 +241,33 @@ describe("validateAttachmentMimeType - Content-Type header bypass security test" }) describe("validateAttachmentMimeType - Unit tests", () => { - it("should return false when target is not an attachments entity", async () => { + it("should return false when target is not an attachments entity", () => { const req = { target: { _attachments: { isAttachmentsEntity: false } }, data: { content: "test" }, reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).not.toHaveBeenCalled() }) - it("should return false when there is no content", async () => { + it("should return false when there is no content", () => { const req = { target: { _attachments: { isAttachmentsEntity: true } }, data: {}, reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).not.toHaveBeenCalled() }) - it("should reject when mimeType does not match acceptable media types", async () => { + it("should reject when mimeType does not match acceptable media types", () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -284,7 +284,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(false) expect(req.reject).toHaveBeenCalledWith( @@ -296,7 +296,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { ) }) - it("should return true when mimeType matches acceptable media types", async () => { + it("should return true when mimeType matches acceptable media types", () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -313,13 +313,13 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() }) - it("should allow any mimeType when @Core.AcceptableMediaTypes is not defined (defaults to */*)", async () => { + it("should allow any mimeType when @Core.AcceptableMediaTypes is not defined (defaults to */*)", () => { const req = { target: { _attachments: { isAttachmentsEntity: true }, @@ -334,7 +334,7 @@ describe("validateAttachmentMimeType - Unit tests", () => { reject: jest.fn(), } - const result = await validateAttachmentMimeType(req) + const result = validateAttachmentMimeType(req) expect(result).toBe(true) expect(req.reject).not.toHaveBeenCalled() From 5ff0f84b9ffb8106d1e0c293672bb6db6b4bb8d1 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 25 Mar 2026 08:57:43 +0100 Subject: [PATCH 19/22] Formatting --- lib/generic-handlers.js | 3 +-- lib/plugin.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index ca279846..9d312889 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -362,8 +362,7 @@ function validateAttachmentMimeType(req) { async function validateAndInsertAttachmentFromDBHandler(data, target, req) { if (!data.content) return - if (!(validateAttachmentMimeType({ data, target, reject: req.reject }))) - return + if (!validateAttachmentMimeType({ data, target, reject: req.reject })) return if ( !(await validateAttachmentSize({ data, target, reject: req.reject }, true)) ) diff --git a/lib/plugin.js b/lib/plugin.js index 337a4591..5036af2a 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -188,7 +188,7 @@ cds.ApplicationService.handle_attachments = cds.service.impl(async function () { } } // Validate mimeType against @Core.AcceptableMediaTypes before uploading content - if (!(validateAttachmentMimeType(req))) return + if (!validateAttachmentMimeType(req)) return const AttachmentsSrv = await cds.connect.to("attachments") try { return await AttachmentsSrv.put(req.target, req.data) From 1ba10c0d6b57a89c428d270a519dbeda9cee3381 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 25 Mar 2026 11:18:02 +0100 Subject: [PATCH 20/22] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b71450..7a6b9add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). The format is based on [Keep a Changelog](http://keepachangelog.com/). +## [Upcoming] Version 3.11.0 + +### Feater + +- Emit the following security events on the attachments service: - AttachmentDownloadRejected, AttachmentSizeExceeded AttachmentUploadRejected +- If `@cap-js/audit-logging` is installed automatically trigger audit logs for the security events + ## Version 3.10.0 ### Fixed From c2ff61ee4192432c723a072f3c55cb64ea5a09eb Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 25 Mar 2026 11:18:16 +0100 Subject: [PATCH 21/22] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a6b9add..5da68652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). ## [Upcoming] Version 3.11.0 -### Feater +### Added - Emit the following security events on the attachments service: - AttachmentDownloadRejected, AttachmentSizeExceeded AttachmentUploadRejected - If `@cap-js/audit-logging` is installed automatically trigger audit logs for the security events From 800e0ec61b92bd19e2f5d70961a7c794423df1e3 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 25 Mar 2026 11:24:35 +0100 Subject: [PATCH 22/22] Update CHANGELOG.md --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5da68652..de20cfe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,13 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). The format is based on [Keep a Changelog](http://keepachangelog.com/). -## [Upcoming] Version 3.11.0 +## [Upcoming] Version 3.10.0 ### Added - Emit the following security events on the attachments service: - AttachmentDownloadRejected, AttachmentSizeExceeded AttachmentUploadRejected - If `@cap-js/audit-logging` is installed automatically trigger audit logs for the security events -## Version 3.10.0 - ### Fixed - Fixed bug where deeply nested attachments were not properly handled.