Add audit logging for sec events

CHANGELOG.md

@@ -4,7 +4,12 @@ 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/).
 
-## Version 3.10.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
 
 ### Fixed
 

README.md

@@ -242,6 +242,23 @@ 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.

lib/generic-handlers.js

@@ -151,13 +151,15 @@ 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")
             await srv.emit("AttachmentDownloadRejected", {
               target: req.target.name,
               keys: { ID: attachmentId },
               status,
+              ipAddress,
             })
           } catch (err) {
             LOG.error("Failed to emit AttachmentDownloadRejected", err)
@@ -274,6 +276,22 @@ 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")
+        await AttachmentsSrv.emit("AttachmentSizeExceeded", {
+          target: req.target.name,
+          keys: req.data.ID ? { ID: req.data.ID } : {},
+          filename: attachmentRef?.filename ?? "n/a",
+          fileSize: length,
+          maxFileSize,
+          ipAddress,
+        })
+      } catch (err) {
+        LOG.error("Failed to emit AttachmentSizeExceeded", err)
+      }
+    })
 
     cds.spawn(async () => {
       try {
@@ -316,6 +334,7 @@ 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")
@@ -326,6 +345,7 @@ function validateAttachmentMimeType(req) {
           mimeType,
           acceptableMediaTypes,
           reason: `MIME type '${mimeType}' is not in @Core.AcceptableMediaTypes`,
+          ipAddress,
         })
       } catch (err) {
         LOG.error("Failed to emit AttachmentUploadRejected", err)

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,

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,
@@ -46,6 +47,26 @@ class AttachmentsService extends cds.Service {
         )
       }
     })
+
+    if (cds.env.requires["audit-log"]) {
+      DEBUG && DEBUG(`Register audit logging handlers for security events.`)
+      this.on(
+        [
+          "AttachmentDownloadRejected",
+          "AttachmentSizeExceeded",
+          "AttachmentUploadRejected",
+        ],
+        async (msg) => {
+          const audit = await cds.connect.to("audit-log")
+          const { ipAddress, ...eventData } = msg.data
+          await audit.log("SecurityEvent", {
+            data: { event: msg.event, ...eventData },
+            ip: ipAddress || undefined,
+          })
+        },
+      )
+    }
+
     return super.init()
   }
 
@@ -276,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,
         )
@@ -292,7 +313,7 @@ class AttachmentsService extends cds.Service {
         )
       }
     }
-    LOG.info("[deleteAttachmentsWithKeys] Finished")
+    LOG.debug("[deleteAttachmentsWithKeys] Finished")
   }
 
   /**

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,

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",
     "@cap-js/postgres": "^2.2.0"
   },
@@ -13,6 +14,10 @@
       }
     },
     "requires": {
+      "queue": {
+        "parallel": true,
+        "chunkSize": 20
+      },
       "attachments": {
         "scanExpiryMs": 30000
       },

tests/unit/auditLogging.test.js

@@ -0,0 +1,108 @@
+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 is disabled", () => {
+  const log = cds.test.log()
+
+  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
+
+    // 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:")
+    cds.env.requires["audit-log"] = originalLog
+  })
+})

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))