Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -24,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
Expand Down
20 changes: 17 additions & 3 deletions lib/generic-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,23 @@ 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, {
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") {
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 }
}

Expand Down
10 changes: 8 additions & 2 deletions lib/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
4 changes: 3 additions & 1 deletion srv/attachments/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/incidents-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "*"
Expand Down
46 changes: 46 additions & 0 deletions tests/integration/attachments-non-draft.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/rejectionEvents.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
})
Loading