From 5810d9ae6897afb3a720ea1f55c29f5b34b279a8 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 12:37:43 +0530 Subject: [PATCH 1/7] JSON error shown in UI instead of user-friendly message This will allow UI5 to extract and display a more readable message, and you avoid raw JSON exposure. --- lib/plugin.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/plugin.js b/lib/plugin.js index acfe93d8..fede0470 100644 --- a/lib/plugin.js +++ b/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, + }); } } } From e08e5558873453e7867bfe4d8a9d1b15c09447ea Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 12:39:10 +0530 Subject: [PATCH 2/7] refactor: improve error handling and logging in scanRequest --- lib/malwareScanner.js | 63 ++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/lib/malwareScanner.js b/lib/malwareScanner.js index fbead11d..ab65a308 100644 --- a/lib/malwareScanner.js +++ b/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, 'Malware scanning service is currently unavailable.') + 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) From 9deac725d15e6fcfb1fb36ed2eeb1c4e7d597516 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 14:25:48 +0530 Subject: [PATCH 3/7] Change scan invocation to use Promise.allSettled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensures one failed scan doesn’t block others and allows better error handling with per-scan logging. --- lib/basic.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/basic.js b/lib/basic.js index 841bff45..7dc1589e 100644 --- a/lib/basic.js +++ b/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; } From 4817df186fe12c5ffcc9991e92872eb7091a9042 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 14:42:26 +0530 Subject: [PATCH 4/7] fix: add fallback check in getFields() for missing attachment keys --- lib/basic.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/basic.js b/lib/basic.js index 7dc1589e..41079494 100644 --- a/lib/basic.js +++ b/lib/basic.js @@ -80,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 From 0b54aa75a603829de5e7abe68a817bf8536c3687 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 15:29:05 +0530 Subject: [PATCH 5/7] test: add validation for user-friendly error when fetching content of flagged attachments --- tests/integration/attachments.test.js | 43 +++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index f7c95933..ffeedd3b 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -158,6 +158,49 @@ describe("Tests for uploading/deleting attachments through API calls - in-memory `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content` )).to.be.rejectedWith(/404/); }); + + it("Uploading attachment - scan fails and returns user-friendly error", async () => { + cds.env.requires.attachments.scan = true; // Ensure scanning is enabled + + // Mock scanRequest to throw + const originalScan = require("@cap-js/attachments/lib/scan"); + jest.spyOn(originalScan, 'scanRequest').mockImplementation(() => { + throw new Error("Simulated malware scan failure"); + }); + + const action = await POST.bind( + {}, + `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=false)/attachments`, + { + up__ID: incidentID, + filename: "fail-scan.pdf", + mimeType: "application/pdf", + content: createReadStream(join(__dirname, "content/sample.pdf")), + createdAt: new Date(), + createdBy: "alice", + } + ); + + try { + await utils.draftModeActions( + "processor", + "Incidents", + incidentID, + "ProcessorService", + action + ); + // If no error is thrown, test should fail + expect.fail("Expected upload to fail due to scan error"); + } catch (err) { + // ✅ Check for friendly error message returned by put() + expect(err.response?.status).to.equal(500); + expect(err.response?.data?.error?.message).to.include("Scan failed for uploaded attachment"); + } + + // Restore original function + originalScan.scanRequest.mockRestore(); +}); + }); describe("Tests for attachments facet disable", () => { From e6a1b0ae1377c1c83f096ac5ad91e38f4ca7fcc6 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Mon, 21 Jul 2025 15:33:08 +0530 Subject: [PATCH 6/7] Revert "test: add validation for user-friendly error when fetching content of flagged attachments" This reverts commit 0b54aa75a603829de5e7abe68a817bf8536c3687. --- tests/integration/attachments.test.js | 43 --------------------------- 1 file changed, 43 deletions(-) diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index ffeedd3b..f7c95933 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -158,49 +158,6 @@ describe("Tests for uploading/deleting attachments through API calls - in-memory `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content` )).to.be.rejectedWith(/404/); }); - - it("Uploading attachment - scan fails and returns user-friendly error", async () => { - cds.env.requires.attachments.scan = true; // Ensure scanning is enabled - - // Mock scanRequest to throw - const originalScan = require("@cap-js/attachments/lib/scan"); - jest.spyOn(originalScan, 'scanRequest').mockImplementation(() => { - throw new Error("Simulated malware scan failure"); - }); - - const action = await POST.bind( - {}, - `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=false)/attachments`, - { - up__ID: incidentID, - filename: "fail-scan.pdf", - mimeType: "application/pdf", - content: createReadStream(join(__dirname, "content/sample.pdf")), - createdAt: new Date(), - createdBy: "alice", - } - ); - - try { - await utils.draftModeActions( - "processor", - "Incidents", - incidentID, - "ProcessorService", - action - ); - // If no error is thrown, test should fail - expect.fail("Expected upload to fail due to scan error"); - } catch (err) { - // ✅ Check for friendly error message returned by put() - expect(err.response?.status).to.equal(500); - expect(err.response?.data?.error?.message).to.include("Scan failed for uploaded attachment"); - } - - // Restore original function - originalScan.scanRequest.mockRestore(); -}); - }); describe("Tests for attachments facet disable", () => { From 5e21d0961a124d4bb47da153bf73a779583b16d4 Mon Sep 17 00:00:00 2001 From: Roshni Naveena S Date: Thu, 24 Jul 2025 11:21:32 +0530 Subject: [PATCH 7/7] cdzcdsvc --- lib/malwareScanner.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/malwareScanner.js b/lib/malwareScanner.js index ab65a308..6eb8b0f8 100644 --- a/lib/malwareScanner.js +++ b/lib/malwareScanner.js @@ -60,7 +60,7 @@ async function scanRequest(Attachments, key, req) { DEBUG?.('[SCAN] Malware scanner responded') } catch (err) { log.error('[SCAN][ScannerError]', err) - req?.error?.(502, 'Malware scanning service is currently unavailable.') + req?.error?.(502, err) await updateStatus(AttachmentsSrv, key, "Failed", currEntity, draftEntity, activeEntity) return }