From 99bf7d934ecb2d295dd5966b12f3879ddb37e4df Mon Sep 17 00:00:00 2001 From: Eric Peairs Date: Tue, 8 Sep 2026 12:20:39 -0600 Subject: [PATCH 1/5] fix: wrong error message when file not yet uploaded --- lib/generic-handlers.js | 13 ++++++- package.json | 2 +- srv/attachments/basic.js | 4 +- tests/unit/rejectionEvents.test.js | 59 ++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 0c666880..ded57463 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -161,9 +161,20 @@ async function onPrepareAttachment(req) { async function getScanInfo(req, reqUrl, AttachmentsSrv) { if (req.target._attachments.isAttachmentsEntity) { const id = req.data.ID || req.params?.at(-1).ID - const { status, lastScan } = await AttachmentsSrv.getStatus(req.target, { + const { status, lastScan, url } = await AttachmentsSrv.getStatus(req.target, { ID: id, }) + // Only when Unscanned: verify a file was actually uploaded (not just metadata from POST). + // For object-store url is set on upload; for db-based url is null so check content existence. + if (status === "Unscanned") { + const hasContent = + url != null || + !!(await SELECT.one + .from(req.target, { ID: id }) + .columns("ID") + .where({ content: { "!=": null } })) + if (!hasContent) return { status: null, lastScan: null, attachmentId: id } + } return { status, lastScan, attachmentId: id } } diff --git a/package.json b/package.json index 6a9dbc15..8d9c6742 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "@aws-sdk/lib-storage": "^3.993.0", "@azure/storage-blob": "^12.31.0", "@google-cloud/storage": "^7.19.0", - "@cap-js/cds-test": "^1", + "@cap-js/cds-test": "^1.0.2", "@cap-js/hana": ">=2.7", "@cap-js/sqlite": ">=2" }, diff --git a/srv/attachments/basic.js b/srv/attachments/basic.js index fab7c0a5..8091f6f8 100644 --- a/srv/attachments/basic.js +++ b/srv/attachments/basic.js @@ -319,16 +319,18 @@ class AttachmentsService extends cds.Service { * Retrieves the malware scan status of an attachment * @param {import('@sap/cds').Entity} Attachments - Attachments entity definition * @param {string} key - The key of the attachment to retrieve the status for - * @returns {{ status: string, lastScan: Date }} - The malware scan status of the attachment + * @returns {{ status: string, lastScan: Date, url: string|null }} - The malware scan status of the attachment */ async getStatus(Attachments, key) { const result = await SELECT.from(Attachments, key).columns([ "status", "lastScan", + "url", ]) return { status: result?.status, lastScan: result?.lastScan, + url: result?.url, } } diff --git a/tests/unit/rejectionEvents.test.js b/tests/unit/rejectionEvents.test.js index 19b1a703..2da4b46d 100644 --- a/tests/unit/rejectionEvents.test.js +++ b/tests/unit/rejectionEvents.test.js @@ -476,6 +476,7 @@ describe("Rescan triggered for Unscanned attachment", () => { attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ status: "Unscanned", lastScan: null, + url: "https://example.com/file.pdf", }) const attachmentId = cds.utils.uuid() @@ -509,6 +510,7 @@ describe("Rescan triggered for Unscanned attachment", () => { attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ status: "Unscanned", lastScan: null, + url: "https://example.com/file.pdf", }) const attachmentId = cds.utils.uuid() @@ -536,6 +538,7 @@ describe("Rescan triggered for Unscanned attachment", () => { attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ status: "Unscanned", lastScan: null, + url: "https://example.com/file.pdf", }) const attachmentId = cds.utils.uuid() @@ -576,6 +579,7 @@ describe("Rescan triggered for Unscanned attachment", () => { attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ status: "Unscanned", lastScan: null, + url: "https://example.com/file.pdf", }) let spawnedFn @@ -611,3 +615,58 @@ describe("Rescan triggered for Unscanned attachment", () => { }) }) }) + +describe("Download rejected when no file has been uploaded", () => { + it("should return 404 when attachment record exists but no file was uploaded", async () => { + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Unscanned", + lastScan: null, + url: null, + }) + + 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(404) + expect(cds.connect.to).not.toHaveBeenCalledWith("malwareScanner") + }) + + it("should return 404 when no file was uploaded even when scan is disabled", async () => { + const target = cds.model.definitions["AdminService.Incidents.attachments"] + + attachmentsSvc.getStatus = jest.fn().mockResolvedValue({ + status: "Unscanned", + lastScan: null, + url: null, + }) + + 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: false } + + await require("../../lib/generic-handlers").validateAttachment(req) + + expect(req.reject).toHaveBeenCalledWith(404) + }) +}) From cba1d1d58d2eceb2502ea571edda80e5c659d639 Mon Sep 17 00:00:00 2001 From: Eric Peairs Date: Wed, 9 Sep 2026 07:34:55 -0600 Subject: [PATCH 2/5] prettier --- lib/generic-handlers.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index ded57463..ef3ff11f 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -161,9 +161,12 @@ async function onPrepareAttachment(req) { async function getScanInfo(req, reqUrl, AttachmentsSrv) { if (req.target._attachments.isAttachmentsEntity) { const id = req.data.ID || req.params?.at(-1).ID - const { status, lastScan, url } = await AttachmentsSrv.getStatus(req.target, { - ID: id, - }) + const { status, lastScan, url } = await AttachmentsSrv.getStatus( + req.target, + { + ID: id, + }, + ) // Only when Unscanned: verify a file was actually uploaded (not just metadata from POST). // For object-store url is set on upload; for db-based url is null so check content existence. if (status === "Unscanned") { From 8c2af695a6d1a33426a93529b46d9319bc0bb6cd Mon Sep 17 00:00:00 2001 From: Eric Peairs Date: Wed, 9 Sep 2026 09:31:43 -0600 Subject: [PATCH 3/5] fix:postgres --- tests/incidents-app/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/incidents-app/package.json b/tests/incidents-app/package.json index e668d4e3..6d1c8e6d 100644 --- a/tests/incidents-app/package.json +++ b/tests/incidents-app/package.json @@ -5,7 +5,7 @@ "@cap-js/attachments": "file:../../.", "@cap-js/audit-logging": "^1.2.0", "@cap-js/hana": ">=2.6.0", - "@cap-js/postgres": "^2.2.0" + "@cap-js/postgres": ">=2.2.0" }, "devDependencies": { "@cap-js/sqlite": "*" From e28cb5f075a3b2f80b7704fbd49f72f412220988 Mon Sep 17 00:00:00 2001 From: Eric Peairs Date: Wed, 9 Sep 2026 09:36:49 -0600 Subject: [PATCH 4/5] chore:changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18cd117c..f05d636c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). ### Fixed - Malware-scan-status gate bypassed by `/content/$value`: `validateAttachment` now recognises the OData `/$value` suffix as a content request and enforces scan policy accordingly. The `getScanInfo` prefix extraction is also corrected for `/$value` URLs (CWE-184). +- Downloading or rescanning a metadata-only attachment (POST created but content not yet uploaded) now correctly returns 404 instead of triggering a rescan with a misleading 202 response. ## Version 4.0.0 - 2026-08-03 From 5b8b08d6e0c3d79c47eef650e72109fd51618bb6 Mon Sep 17 00:00:00 2001 From: Eric Peairs Date: Thu, 10 Sep 2026 08:30:43 -0600 Subject: [PATCH 5/5] add:duplicate key error fix --- CHANGELOG.md | 6 +++ lib/plugin.js | 10 +++- .../integration/attachments-non-draft.test.js | 46 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f05d636c..546a8bcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). - Attachments are now served with `Content-Disposition: attachment` by default with inline being a toggle +## Version 3.13.5 - [Unreleased] + +### Fixed + +- Programmatic attachment imports that include file content no longer fail with a duplicate-key error on HANA. + ## Version 3.13.4 - 2026-07-31 ### Fixed diff --git a/lib/plugin.js b/lib/plugin.js index 02052334..2e5ab1c6 100644 --- a/lib/plugin.js +++ b/lib/plugin.js @@ -62,8 +62,14 @@ cds.once("served", () => { await validateAndInsertAttachmentFromDBHandler(entry, req.target, req) } - // Return the data as the result (attachment.put handles the actual storage) - return next() + // For composition attachments, attachment.put() already UPSERTs to DB — calling + // next() would cause a duplicate INSERT. Inline attachments need next() to persist + // the parent record's metadata fields (url, filename, etc.). + return req.target._attachments.isAttachmentsEntity + ? entries.length === 1 + ? entries[0] + : entries + : next() }) /** diff --git a/tests/integration/attachments-non-draft.test.js b/tests/integration/attachments-non-draft.test.js index e69b18fb..bc278399 100644 --- a/tests/integration/attachments-non-draft.test.js +++ b/tests/integration/attachments-non-draft.test.js @@ -468,6 +468,52 @@ describe("Tests for uploading/deleting and fetching attachments through API call expect(await deletion).toBe(true) }) + it("Programmatic INSERT with attachment content should not cause duplicate key error", async () => { + const incidentID = cds.utils.uuid() + const attachmentID = cds.utils.uuid() + await INSERT.into("sap.capire.incidents.Incidents").entries({ + ID: incidentID, + title: "Programmatic import test", + }) + + const AttachmentsSrv = await cds.connect.to("attachments") + const target = + cds.model.definitions["sap.capire.incidents.Incidents.attachments"] + const putSpy = jest + .spyOn(AttachmentsSrv, "put") + .mockImplementation(async (_t, data) => { + await UPSERT.into(target).entries({ + up__ID: data.up__ID, + ID: data.ID, + url: data.url, + filename: data.filename, + mimeType: data.mimeType, + status: "Unscanned", + }) + }) + const originalKind = cds.env.requires.attachments.kind + cds.env.requires.attachments.kind = "aws-s3" + + try { + const { Readable } = require("stream") + await INSERT.into(target).entries({ + up__ID: incidentID, + ID: attachmentID, + filename: "import.pdf", + mimeType: "application/pdf", + content: Readable.from(Buffer.from("pdf")), + }) + } finally { + cds.env.requires.attachments.kind = originalKind + putSpy.mockRestore() + } + + const db = await cds.connect.to("db") + const rows = await db.run(SELECT.from(target).where({ up__ID: incidentID })) + expect(rows.length).toBe(1) + expect(rows[0].ID).toBe(attachmentID) + }) + it("Should create NonDraftTest entities using programmatic INSERT and add attachments", async () => { const firstID = cds.utils.uuid() const secondID = cds.utils.uuid()