Improve error handling and logging in attachment scan flow

lib/basic.js

@@ -25,7 +25,15 @@ module.exports = class AttachmentsService extends cds.Service {
       );
     }
 
-    if(this.kind === 'db') data.map((d) => { scanRequest(attachments, { ID: d.ID })})
+    if (this.kind === "db") {
+      await Promise.allSettled(
+        data.map((d) =>
+          scanRequest(attachments, { ID: d.ID }).catch((e) =>
+            DEBUG?.("Scan failed", e)
+          )
+        )
+      );
+    }
 
     return res;
   }
@@ -72,6 +80,7 @@ module.exports = class AttachmentsService extends cds.Service {
 
   getFields(attachments) {
     const attachmentFields = ["filename", "mimeType", "content", "url", "ID"];
+    if (!attachments?.keys) throw new Error("Invalid attachment entity: missing keys");
     const { up_ } = attachments.keys;
     if (up_)
       return up_.keys

lib/malwareScanner.js

@@ -5,6 +5,8 @@ const { SELECT } = cds.ql;
 async function scanRequest(Attachments, key, req) {
   const scanEnabled = cds.env.requires?.attachments?.scan ?? true
   const AttachmentsSrv = await cds.connect.to("attachments")
+  const log = cds.log('attachments')
+  const start = Date.now()
 
   let draftEntity, activeEntity
   if (Attachments.isDraft) {
@@ -14,36 +16,38 @@ async function scanRequest(Attachments, key, req) {
     activeEntity = Attachments
   }
 
-  let currEntity = draftEntity == undefined ? activeEntity : draftEntity
+  const currEntity = draftEntity ?? activeEntity
 
   if (!scanEnabled) {
     if (cds.env.profiles.some(p => p === "development" || p === "test") && !cds.env.profiles.includes("hybrid")) {
       await updateStatus(AttachmentsSrv, key, "Scanning", currEntity, draftEntity, activeEntity)
       setTimeout(() => {
-        DEBUG?.('Malware scanning is disabled. Setting scan status to Clean in development profile.')
+        DEBUG?.('[SCAN] Scanning disabled: setting status to Clean (dev/test profile)')
         updateStatus(AttachmentsSrv, key, "Clean", currEntity, draftEntity, activeEntity)
-          .catch(e => cds.log('attachments').error(e))
+          .catch(e => log.error('[SCAN][DevFallback]', e))
       }, 5000).unref()
-      return
-    } else {
-      return
     }
+    return
   }
 
   await updateStatus(AttachmentsSrv, key, "Scanning", currEntity, draftEntity, activeEntity)
 
-  const credentials = getCredentials()
-  const contentStream = await AttachmentsSrv.get(currEntity, key)
+  // Read file content
   let fileContent
   try {
+    const contentStream = await AttachmentsSrv.get(currEntity, key)
     fileContent = await streamToString(contentStream)
+    DEBUG?.('[SCAN] File content read successfully')
   } catch (err) {
-    DEBUG?.("Malware Scanning: Cannot read file content", err)
+    log.error('[SCAN][ReadError]', err)
+    req?.error?.(409, 'Unable to read file for malware scan.')
     await updateStatus(AttachmentsSrv, key, "Failed", currEntity, draftEntity, activeEntity)
     return
   }
 
-  let response;
+  // Make request to malware scanner
+  const credentials = getCredentials()
+  let response
   try {
     response = await fetch(`https://${credentials.uri}/scan`, {
       method: "POST",
@@ -53,26 +57,53 @@ async function scanRequest(Attachments, key, req) {
       },
       body: fileContent,
     })
-  } catch (error) {
-    DEBUG?.("Request to malware scanner failed", error)
+    DEBUG?.('[SCAN] Malware scanner responded')
+  } catch (err) {
+    log.error('[SCAN][ScannerError]', err)
+    req?.error?.(502, err)
+    await updateStatus(AttachmentsSrv, key, "Failed", currEntity, draftEntity, activeEntity)
+    return
+  }
+
+  // Parse response
+  let responseText
+  try {
+    responseText = await response.json()
+    DEBUG?.('[SCAN] Malware scanner response parsed', responseText)
+  } catch (err) {
+    log.error('[SCAN][ResponseParseError]', err)
+    req?.error?.(500, 'Invalid response from malware scanner.')
     await updateStatus(AttachmentsSrv, key, "Failed", currEntity, draftEntity, activeEntity)
     return
   }
 
+  // Interpret result and update
   try {
-    const responseText = await response.json()
     const status = responseText.malwareDetected ? "Infected" : "Clean"
+    DEBUG?.(`[SCAN] Final scan result: ${status}`)
+
     if (status === "Infected") {
-      DEBUG?.("Malware detected in the file, deleting attachment content from db", key)
+      DEBUG?.(`[SCAN] Infected file detected, deleting content for key: ${key}`)
       await AttachmentsSrv.deleteInfectedAttachment(currEntity, key, req)
+      req?.notify?.(200, 'Malware detected. Attachment was deleted.')
+    } else {
+      req?.notify?.(200, 'Attachment passed malware scan.')
     }
+
     await updateStatus(AttachmentsSrv, key, status, currEntity, draftEntity, activeEntity)
+
+    log.info('[SCAN][Complete]', {
+      key,
+      status,
+      timeMs: Date.now() - start
+    })
+
   } catch (err) {
-    DEBUG?.("Cannot serialize malware scanner response body", err)
+    log.error('[SCAN][StatusUpdateError]', err)
+    req?.error?.(500, 'Scan complete but failed to update final status.')
     await updateStatus(AttachmentsSrv, key, "Failed", currEntity, draftEntity, activeEntity)
   }
 }
-
 async function updateStatus(AttachmentsSrv, key, status, currEntity, draftEntity, activeEntity) {
   if (currEntity == draftEntity) {
     currEntity = await getCurrentEntity(currEntity, activeEntity, key)

lib/plugin.js

@@ -92,8 +92,12 @@ cds.once("served", async function registerPluginHandlers () {
       const attachmentID = req.req.url.match(attachmentIDRegex)[1]
       const status = await AttachmentsSrv.getStatus(req.target, { ID: attachmentID })
       const scanEnabled = cds.env.requires?.attachments?.scan ?? true
-      if (scanEnabled && status !== 'Clean') {
-        req.reject(403, 'Unable to download the attachment as scan status is not clean.')
+      if (scanEnabled && status !== "Clean") {
+        req.reject(403, {
+          message:
+            "Attachment is still being scanned for malware. Please try again later.",
+          target: req.target.name,
+        });
       }
     }
   }