From b6976d9a569bcd0f7f4118fd04907c1f163a7e73 Mon Sep 17 00:00:00 2001 From: Christian Schuerings Date: Mon, 16 Mar 2026 15:34:00 +0200 Subject: [PATCH 01/17] feat: add native server-side copy() to AttachmentsService Introduce a copy() method on AttachmentsService that duplicates an attachment record and its binary content across active and draft tables. Includes storage-level implementations for AWS S3, Azure Blob Storage, and GCP, with safeguards against targetKeys override, entity injection, and infected/failed source attachments. --- CHANGELOG.md | 6 + README.md | 78 +++++++ srv/aws-s3.js | 38 ++++ srv/azure-blob-storage.js | 33 +++ srv/basic.js | 168 +++++++++++++++ srv/gcp.js | 45 ++++- tests/integration/attachments.test.js | 281 ++++++++++++++++++++++++++ 7 files changed, 647 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0caa0ad..832c9846 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +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/). +## Unreleased + +### Added + +- Native server-side `copy()` method on `AttachmentsService` for copying attachments between entities without transferring binary data through the application. Supports all storage backends (DB, AWS S3, Azure Blob Storage, GCP Cloud Storage) with backend-native copy operations. + ## Version 3.9.0 ### Fixed diff --git a/README.md b/README.md index 6d1732fc..71bdb6e4 100755 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ The `@cap-js/attachments` package is a [CDS plugin](https://cap.cloud.sap/docs/n - [Visibility Control for Attachments UI Facet Generation](#visibility-control-for-attachments-ui-facet-generation) - [Example Usage](#example-usage) - [Non-Draft Upload](#non-draft-upload) + - [Copying Attachments](#copying-attachments) - [Specify the maximum file size](#specify-the-maximum-file-size) - [Restrict allowed MIME types](#restrict-allowed-mime-types) - [Minimum and Maximum Number of Attachments](#minimum-and-maximum-number-of-attachments) @@ -272,6 +273,83 @@ entity Incidents { } ``` +### Copying Attachments + +The `AttachmentsService` exposes a programmatic `copy()` method that copies an attachment to a new record. On cloud storage backends (AWS S3, Azure Blob Storage, GCP Cloud Storage) this uses a backend-native server-side copy — no binary data is transferred through your application. On database storage it reads and inserts the content directly. + +```js +const AttachmentsSrv = await cds.connect.to("attachments") + +await AttachmentsSrv.copy( + sourceAttachments, + { ID: sourceAttachmentID }, + targetAttachments, + { up__ID: targetParentID }, +) +``` + +**Signature:** + +```js +copy(sourceAttachments, sourceKeys, targetAttachments, (targetKeys = {})) +``` + +| Parameter | Description | +| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sourceAttachments` | CDS entity definition of the source attachment composition. Pass `sourceAttachments.drafts` to copy from a draft-only source. | +| `sourceKeys` | Keys identifying the source attachment (e.g. `{ ID: '...' }`) | +| `targetAttachments` | CDS entity definition of the target attachment composition. Pass `targetAttachments.drafts` when the target parent is in an open draft editing session — the new record will be inserted into the draft shadow table and committed on save. | +| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID` (the draft session UUID of the target parent). | + +The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment with status `Infected` or `Failed` is rejected with a `400` error. + +> **Note:** Only copies within the same tenant are supported. Cross-tenant copies are not possible. + +> **Note:** `copy()` uses raw CQL internally and does not enforce CDS service-level authorization (`@requires` / `@restrict`). Callers are responsible for verifying that the current user has appropriate access to both the source and target entities before invoking `copy()`. + +**Supported scenarios:** + +| Source | Target | Description | +| -------------------- | -------------------- | ------------------------------------------------------------------------------------------------------ | +| `Attachments` | `Attachments` | Copy between two active records, e.g. duplicating an incident. | +| `Attachments` | `Attachments.drafts` | Copy into a record that is in an open draft session, e.g. initialising a new incident from a template. | +| `Attachments.drafts` | `Attachments.drafts` | Copy from a draft-only source (e.g. a template that has never been activated) into another draft. | + +**Example — copy between two active records:** + +```js +const { Incidents } = ProcessorService.entities + +await AttachmentsSrv.copy( + Incidents.attachments, + { ID: sourceAttachmentID }, + Incidents.attachments, + { up__ID: targetIncidentID }, +) +``` + +**Example — copy into a new draft record (e.g. creating an incident from a template):** + +```js +const { Incidents } = ProcessorService.entities + +// Look up the draft session UUID for the target incident's open draft +const targetDraft = await SELECT.one + .from(Incidents.drafts, { ID: targetIncidentID }) + .columns("DraftAdministrativeData_DraftUUID") + +await AttachmentsSrv.copy( + Incidents.attachments, + { ID: sourceAttachmentID }, + Incidents.attachments.drafts, + { + up__ID: targetIncidentID, + DraftAdministrativeData_DraftUUID: + targetDraft.DraftAdministrativeData_DraftUUID, + }, +) +``` + ### Non-Draft Upload For scenarios where the entity is not draft-enabled, for example [`tests/non-draft-request.http`](./tests/non-draft-request.http), separate HTTP requests for metadata creation and asset uploading need to be performed manually. diff --git a/srv/aws-s3.js b/srv/aws-s3.js index 7afe1592..c4ab386d 100644 --- a/srv/aws-s3.js +++ b/srv/aws-s3.js @@ -3,6 +3,7 @@ const { GetObjectCommand, DeleteObjectCommand, HeadObjectCommand, + CopyObjectCommand, } = require("@aws-sdk/client-s3") const { Upload } = require("@aws-sdk/lib-storage") const cds = require("@sap/cds") @@ -328,6 +329,43 @@ module.exports = class AWSAttachmentsService extends require("./object-store") { } } + /** + * @inheritdoc + */ + async copy( + sourceAttachments, + sourceKeys, + targetAttachments, + targetKeys = {}, + ) { + LOG.debug("Copying attachment (S3)", { + source: sourceAttachments.name, + sourceKeys, + target: targetAttachments.name, + }) + const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) + const { source, newID, newUrl } = await this._prepareCopy( + sourceAttachments, + sourceKeys, + ) + const { client, bucket } = await this.retrieveClient() + if (await this.exists(newUrl)) { + const err = new Error("Target blob already exists") + err.status = 409 + throw err + } + await client.send( + new CopyObjectCommand({ + Bucket: bucket, + CopySource: `${bucket}/${source.url}`, + Key: newUrl, + }), + ) + const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + await INSERT(newRecord).into(targetAttachments) + return newRecord + } + /** * Deletes a file from S3 based on the provided key * @param {string} Key - The key of the file to delete diff --git a/srv/azure-blob-storage.js b/srv/azure-blob-storage.js index 11e5128d..8554e33b 100644 --- a/srv/azure-blob-storage.js +++ b/srv/azure-blob-storage.js @@ -305,6 +305,39 @@ module.exports = class AzureAttachmentsService extends ( } } + /** + * @inheritdoc + */ + async copy( + sourceAttachments, + sourceKeys, + targetAttachments, + targetKeys = {}, + ) { + LOG.debug("Copying attachment (Azure)", { + source: sourceAttachments.name, + sourceKeys, + target: targetAttachments.name, + }) + const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) + const { source, newID, newUrl } = await this._prepareCopy( + sourceAttachments, + sourceKeys, + ) + const { containerClient } = await this.retrieveClient() + if (await this.exists(newUrl)) { + const err = new Error("Target blob already exists") + err.status = 409 + throw err + } + const sourceBlobClient = containerClient.getBlockBlobClient(source.url) + const targetBlobClient = containerClient.getBlockBlobClient(newUrl) + await targetBlobClient.syncCopyFromURL(sourceBlobClient.url) + const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + await INSERT(newRecord).into(targetAttachments) + return newRecord + } + /** * Deletes a file from Azure Blob Storage * @param {string} Key - The key of the file to delete diff --git a/srv/basic.js b/srv/basic.js index 4ab063dc..dc869a90 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -12,6 +12,39 @@ class AttachmentsService extends cds.Service { await this.delete(msg.data.url, msg.data.target) }) + this.on("CopyAttachment", async (msg) => { + const { + sourceTarget, + sourceKeys, + targetTarget, + targetKeys, + targetIsDraft = false, + } = msg.data + const sourceAttachments = cds.model.definitions[sourceTarget] + if (!sourceAttachments?.["@_is_media_data"]) { + const err = new Error( + `Invalid source entity: ${sourceTarget} is not an attachment entity`, + ) + err.status = 400 + throw err + } + const targetDef = cds.model.definitions[targetTarget] + if (!targetDef?.["@_is_media_data"]) { + const err = new Error( + `Invalid target entity: ${targetTarget} is not an attachment entity`, + ) + err.status = 400 + throw err + } + const targetAttachments = targetIsDraft ? targetDef?.drafts : targetDef + return this.copy( + sourceAttachments, + sourceKeys, + targetAttachments, + targetKeys, + ) + }) + this.on("DeleteInfectedAttachment", async (msg) => { const { target, hash, keys } = msg.data const attachment = await SELECT.one @@ -464,6 +497,141 @@ class AttachmentsService extends cds.Service { } } + /** + * Fields that are never allowed in targetKeys — they are controlled by the + * copy logic and must not be overridden by callers. + */ + static _PROTECTED_FIELDS = new Set([ + "ID", + "url", + "content", + "hash", + "status", + "lastScan", + "filename", + "mimeType", + "note", + "statusNav", + "createdAt", + "createdBy", + "modifiedAt", + "modifiedBy", + ]) + + /** + * Strips protected fields from targetKeys so callers cannot override + * security-sensitive metadata (status, hash, url, etc.). + * @param {object} targetKeys - Raw target keys from the caller + * @returns {object} - Sanitized target keys containing only FK fields + */ + _sanitizeTargetKeys(targetKeys) { + const sanitized = {} + for (const [key, value] of Object.entries(targetKeys)) { + if (!AttachmentsService._PROTECTED_FIELDS.has(key)) { + sanitized[key] = value + } else { + LOG.warn(`Ignoring protected field in targetKeys: ${key}`) + } + } + return sanitized + } + + /** + * Prepares a copy operation by validating the source and generating new identifiers. + * Shared by all storage backends. + * @param {import('@sap/cds').Entity} sourceAttachments - Source attachment entity definition + * @param {object} sourceKeys - Keys identifying the source attachment (e.g. { ID: '...' }) + * @returns {Promise<{ source: object, newID: string, newUrl: string }>} + */ + async _prepareCopy(sourceAttachments, sourceKeys) { + const source = await SELECT.one + .from(sourceAttachments, sourceKeys) + .columns( + "url", + "filename", + "mimeType", + "note", + "hash", + "status", + "lastScan", + ) + if (!source) { + const err = new Error("Source attachment not found") + err.status = 404 + throw err + } + if (source.status === "Infected" || source.status === "Failed") { + const err = new Error( + `Cannot copy attachment with status: ${source.status}`, + ) + err.status = 400 + throw err + } + const isMultiTenancyEnabled = !!cds.env.requires.multitenancy + const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind + const newUrl = + isMultiTenancyEnabled && objectStoreKind === "shared" + ? `${cds.context.tenant}_${cds.utils.uuid()}` + : cds.utils.uuid() + return { source, newID: cds.utils.uuid(), newUrl } + } + + /** + * Copies an attachment to a new record, reusing the binary content from storage. + * For DB storage, reads content and inserts a new record. + * Cloud backends override this to use native server-side copy. + * Scan status, lastScan, and hash are inherited from the source — no re-scan needed. + * + * Authorization note: this method uses raw CQL and does not enforce CDS + * service-level authorization (@requires / @restrict). Callers are responsible + * for verifying that the current user has read access to the source and write + * access to the target before invoking copy(). + * + * Tenant note: only copies within the same tenant are supported. Cross-tenant + * copies are not allowed because the storage backends resolve credentials for + * the current tenant only. + * + * @param {import('@sap/cds').Entity} sourceAttachments - Source attachment entity definition. + * Pass `sourceAttachments.drafts` to copy from a draft-only source. + * @param {object} sourceKeys - Keys identifying the source attachment (e.g. { ID: '...' }) + * @param {import('@sap/cds').Entity} targetAttachments - Target attachment entity definition. + * Pass `targetAttachments.drafts` to insert into the draft shadow table (i.e. when the target + * parent entity is currently in a draft editing session). In that case targetKeys must include + * DraftAdministrativeData_DraftUUID. + * @param {object} [targetKeys={}] - Parent FK fields for the new record (e.g. { up__ID: '...' }). + * When targeting a draft table, must also include DraftAdministrativeData_DraftUUID. + * Protected fields (status, hash, url, etc.) are stripped automatically. + * @returns {Promise} - New attachment metadata (without content) + */ + async copy( + sourceAttachments, + sourceKeys, + targetAttachments, + targetKeys = {}, + ) { + LOG.debug("Copying attachment (DB)", { + source: sourceAttachments.name, + sourceKeys, + target: targetAttachments.name, + }) + const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) + const { source, newID, newUrl } = await this._prepareCopy( + sourceAttachments, + sourceKeys, + ) + const content = await this.get(sourceAttachments, sourceKeys) + const newRecord = { + ...safeTargetKeys, + ...source, + ID: newID, + url: newUrl, + content, + } + await INSERT(newRecord).into(targetAttachments) + const { content: _, ...metadata } = newRecord + return metadata + } + /** * Deletes a file from the database. Does not delete metadata * @param {string} url - The url of the file to delete diff --git a/srv/gcp.js b/srv/gcp.js index 209a791c..4d1ec1b1 100644 --- a/srv/gcp.js +++ b/srv/gcp.js @@ -104,6 +104,17 @@ module.exports = class GoogleAttachmentsService extends ( } } + /** + * Checks if a file exists in Google Cloud Storage + * @param {string} fileName - The name/key of the file to check + * @returns {Promise} - True if the file exists, false otherwise + */ + async exists(fileName) { + const { bucket } = await this.retrieveClient() + const [exists] = await bucket.file(fileName).exists() + return exists + } + /** * @inheritdoc */ @@ -151,8 +162,7 @@ module.exports = class GoogleAttachmentsService extends ( const file = bucket.file(blobName) - const [exists] = await file.exists() - if (exists) { + if (await this.exists(blobName)) { const error = new Error("Attachment already exists") error.status = 409 throw error @@ -336,6 +346,37 @@ module.exports = class GoogleAttachmentsService extends ( } } + /** + * @inheritdoc + */ + async copy( + sourceAttachments, + sourceKeys, + targetAttachments, + targetKeys = {}, + ) { + LOG.debug("Copying attachment (GCP)", { + source: sourceAttachments.name, + sourceKeys, + target: targetAttachments.name, + }) + const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) + const { source, newID, newUrl } = await this._prepareCopy( + sourceAttachments, + sourceKeys, + ) + const { bucket } = await this.retrieveClient() + if (await this.exists(newUrl)) { + const err = new Error("Target blob already exists") + err.status = 409 + throw err + } + await bucket.file(source.url).copy(bucket.file(newUrl)) + const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + await INSERT(newRecord).into(targetAttachments) + return newRecord + } + /** * Deletes a file from Google Cloud Platform * @param {string} Key - The key of the file to delete diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index 4829c716..6e6be2b1 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -2714,6 +2714,287 @@ describe("Testing to prevent crash due to recursive overflow", () => { }) }) +describe("Tests for copy() on AttachmentsService", () => { + beforeAll(async () => { + utils = new RequestSend(POST) + }) + + it("Copies a clean attachment to a different incident", async () => { + const sourceIncidentID = await newIncident(POST, "processor") + const targetIncidentID = await newIncident(POST, "processor") + const sourceCleanWaiter = waitForScanStatus("Clean") + + const sourceAttachmentID = await uploadDraftAttachment( + utils, + POST, + GET, + sourceIncidentID, + ) + expect(sourceAttachmentID).toBeTruthy() + await utils.draftModeSave( + "processor", + "Incidents", + targetIncidentID, + "ProcessorService", + ) + await sourceCleanWaiter + + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + const newAtt = await AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments, + { up__ID: targetIncidentID }, + ) + expect(newAtt.ID).not.toEqual(sourceAttachmentID) + expect(newAtt.url).toBeTruthy() + expect(newAtt.filename).toEqual("sample.pdf") + expect(newAtt.mimeType).toBeTruthy() + expect(newAtt.hash).toBeTruthy() + // Scan status is inherited from source — no re-scan needed + expect(newAtt.status).toEqual("Clean") + + // Verify the copied record is in the DB under the target incident + const copied = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=true)/attachments`, + ) + expect(copied.status).toEqual(200) + expect(copied.data.value.length).toEqual(1) + expect(copied.data.value[0].ID).toEqual(newAtt.ID) + expect(copied.data.value[0].filename).toEqual("sample.pdf") + + // Verify content is downloadable + const contentResponse = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=true)/attachments(up__ID=${targetIncidentID},ID=${newAtt.ID},IsActiveEntity=true)/content`, + ) + expect(contentResponse.status).toEqual(200) + expect(contentResponse.data).toBeTruthy() + }) + + it("Copies an active attachment into a draft incident (active -> draft)", async () => { + const sourceIncidentID = await newIncident(POST, "processor") + const targetIncidentID = await newIncident(POST, "processor") // starts as draft + const sourceCleanWaiter = waitForScanStatus("Clean") + + const sourceAttachmentID = await uploadDraftAttachment( + utils, + POST, + GET, + sourceIncidentID, + ) + expect(sourceAttachmentID).toBeTruthy() + await sourceCleanWaiter + + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + // Look up the DraftUUID of the target incident's draft session + const targetDraft = await SELECT.one + .from(ProcessorService.entities.Incidents.drafts, { + ID: targetIncidentID, + }) + .columns("DraftAdministrativeData_DraftUUID") + expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() + + const newAtt = await AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments.drafts, + { + up__ID: targetIncidentID, + DraftAdministrativeData_DraftUUID: + targetDraft.DraftAdministrativeData_DraftUUID, + }, + ) + expect(newAtt.ID).toBeTruthy() + expect(newAtt.status).toEqual("Clean") + + // Verify the record exists in the draft table (IsActiveEntity=false) + const draftAttachments = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=false)/attachments`, + ) + expect(draftAttachments.status).toEqual(200) + expect(draftAttachments.data.value.length).toEqual(1) + expect(draftAttachments.data.value[0].ID).toEqual(newAtt.ID) + + // After saving the draft, the attachment should appear in the active entity + await utils.draftModeSave( + "processor", + "Incidents", + targetIncidentID, + "ProcessorService", + ) + const activeAttachments = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=true)/attachments`, + ) + expect(activeAttachments.status).toEqual(200) + expect(activeAttachments.data.value.length).toEqual(1) + expect(activeAttachments.data.value[0].ID).toEqual(newAtt.ID) + + // Content should be downloadable from the active entity + const contentResponse = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=true)/attachments(up__ID=${targetIncidentID},ID=${newAtt.ID},IsActiveEntity=true)/content`, + ) + expect(contentResponse.status).toEqual(200) + expect(contentResponse.data).toBeTruthy() + }) + + it("Copies a draft attachment into another draft incident (draft -> draft)", async () => { + const sourceIncidentID = await newIncident(POST, "processor") // draft + const targetIncidentID = await newIncident(POST, "processor") // draft + const sourceCleanWaiter = waitForScanStatus("Clean") + + // Upload to source as draft, then save it to active so it gets scanned + const sourceAttachmentID = await uploadDraftAttachment( + utils, + POST, + GET, + sourceIncidentID, + ) + expect(sourceAttachmentID).toBeTruthy() + await sourceCleanWaiter + + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + // Look up DraftUUID for target draft session + const targetDraft = await SELECT.one + .from(ProcessorService.entities.Incidents.drafts, { + ID: targetIncidentID, + }) + .columns("DraftAdministrativeData_DraftUUID") + expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() + + // Source is the active Attachments entity (uploaded via draft, now active after save) + const newAtt = await AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments.drafts, + { + up__ID: targetIncidentID, + DraftAdministrativeData_DraftUUID: + targetDraft.DraftAdministrativeData_DraftUUID, + }, + ) + expect(newAtt.ID).toBeTruthy() + expect(newAtt.status).toEqual("Clean") + + // Verify it is visible in draft context + const draftAttachments = await GET( + `odata/v4/processor/Incidents(ID=${targetIncidentID},IsActiveEntity=false)/attachments`, + ) + expect(draftAttachments.status).toEqual(200) + expect(draftAttachments.data.value.length).toEqual(1) + expect(draftAttachments.data.value[0].ID).toEqual(newAtt.ID) + }) + + it("Copy rejects attachment with Infected status", async () => { + const incidentID = await newIncident(POST, "processor") + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + // Directly insert a fake infected attachment record + const infectedID = cds.utils.uuid() + await cds.run( + INSERT({ + ID: infectedID, + url: cds.utils.uuid(), + filename: "infected.pdf", + mimeType: "application/pdf", + status: "Infected", + up__ID: incidentID, + }).into(Attachments), + ) + + await expect( + AttachmentsSrv.copy(Attachments, { ID: infectedID }, Attachments, { + up__ID: incidentID, + }), + ).rejects.toMatchObject({ status: 400 }) + }) + + it("Copy rejects non-existent source attachment", async () => { + const incidentID = await newIncident(POST, "processor") + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + await expect( + AttachmentsSrv.copy(Attachments, { ID: cds.utils.uuid() }, Attachments, { + up__ID: incidentID, + }), + ).rejects.toMatchObject({ status: 404 }) + }) + + it("Copy strips protected fields from targetKeys", async () => { + const sourceIncidentID = await newIncident(POST, "processor") + const targetIncidentID = await newIncident(POST, "processor") + const sourceCleanWaiter = waitForScanStatus("Clean") + + const sourceAttachmentID = await uploadDraftAttachment( + utils, + POST, + GET, + sourceIncidentID, + ) + expect(sourceAttachmentID).toBeTruthy() + await utils.draftModeSave( + "processor", + "Incidents", + targetIncidentID, + "ProcessorService", + ) + await sourceCleanWaiter + + const AttachmentsSrv = await cds.connect.to("attachments") + const { ProcessorService } = cds.services + const Attachments = ProcessorService.entities["Incidents.attachments"] + + // Attempt to override protected fields via targetKeys + const newAtt = await AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments, + { + up__ID: targetIncidentID, + status: "Unscanned", + hash: "tampered-hash", + filename: "evil.exe", + mimeType: "application/x-evil", + }, + ) + + // Protected fields must reflect the source, not the attacker's values + expect(newAtt.status).toEqual("Clean") + expect(newAtt.hash).not.toEqual("tampered-hash") + expect(newAtt.filename).toEqual("sample.pdf") + expect(newAtt.mimeType).not.toEqual("application/x-evil") + }) + + it("CopyAttachment event rejects non-attachment entity names", async () => { + const AttachmentsSrv = cds.unboxed(await cds.connect.to("attachments")) + const { ProcessorService } = cds.services + + // Use a real non-attachment entity name + const nonAttachmentEntity = ProcessorService.entities.Incidents.name + + await expect( + AttachmentsSrv.send("CopyAttachment", { + sourceTarget: nonAttachmentEntity, + sourceKeys: { ID: cds.utils.uuid() }, + targetTarget: nonAttachmentEntity, + targetKeys: {}, + }), + ).rejects.toMatchObject({ status: 400 }) + }) +}) + /** * Uploads attachment in draft mode using CDS test utilities * @param {Object} utils - RequestSend utility instance From 66e45d5fabb55a5846a7c9fa5ee0a11318bb1f3f Mon Sep 17 00:00:00 2001 From: Christian Schuerings Date: Mon, 16 Mar 2026 15:34:00 +0200 Subject: [PATCH 02/17] chore: add Copy Incident action to incidents-app Fiori UI Add a bound copyIncident action on ProcessorService.Incidents that creates a new draft with all attachments cloned via AttachmentsSrv.copy(). The button is shown only in display mode on the Object Page header. --- .../app/incidents/annotations.cds | 11 +++++ tests/incidents-app/srv/services.cds | 4 +- tests/incidents-app/srv/services.js | 49 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/tests/incidents-app/app/incidents/annotations.cds b/tests/incidents-app/app/incidents/annotations.cds index 4c856500..8a3dfcfa 100644 --- a/tests/incidents-app/app/incidents/annotations.cds +++ b/tests/incidents-app/app/incidents/annotations.cds @@ -154,6 +154,17 @@ annotate service.Incidents with { ![@UI.TextArrangement] : #TextOnly, } }; + +annotate service.Incidents with @( + UI.Identification : [{ + $Type : 'UI.DataFieldForAction', + Label : 'Copy Incident', + Action : 'ProcessorService.copyIncident', + ![@UI.Hidden] : { $edmJson: { $Not: { $Path: 'IsActiveEntity' } } }, + InvocationGrouping : #Isolated, + }] +); + annotate service.Incidents.conversation with @( title : '{i18n>Conversation}', UI.LineItem #i18nConversation1 : [ diff --git a/tests/incidents-app/srv/services.cds b/tests/incidents-app/srv/services.cds index 1643c8d9..7072ec74 100644 --- a/tests/incidents-app/srv/services.cds +++ b/tests/incidents-app/srv/services.cds @@ -6,7 +6,9 @@ using from '../db/attachments'; */ service ProcessorService { @cds.redirection.target - entity Incidents as projection on my.Incidents; + entity Incidents as projection on my.Incidents actions { + action copyIncident() returns Incidents; + }; entity Customers @readonly as projection on my.Customers; diff --git a/tests/incidents-app/srv/services.js b/tests/incidents-app/srv/services.js index caf54db2..1038f2a5 100644 --- a/tests/incidents-app/srv/services.js +++ b/tests/incidents-app/srv/services.js @@ -24,10 +24,59 @@ class ProcessorService extends cds.ApplicationService { ) this.on("insertTestData", () => this.insertTestData()) + this.on("copyIncident", (req) => this.onCopyIncident(req)) return res } + async onCopyIncident(req) { + const { Incidents } = this.entities + const Attachments = this.entities["Incidents.attachments"] + const sourceID = req.params[0]?.ID ?? req.params[0] + + // Read source incident fields + const source = await SELECT.one + .from(Incidents, { ID: sourceID }) + .columns("title", "customer_ID", "urgency_code") + if (!source) return req.reject(404, "Source incident not found") + + // Create a new draft incident + const newDraft = await this.new(Incidents.drafts, { + title: source.title + " (Copy)", + customer_ID: source.customer_ID, + urgency_code: source.urgency_code, + }) + + // Look up the DraftUUID for the new draft + const draftAdmin = await SELECT.one + .from(Incidents.drafts, { ID: newDraft.ID }) + .columns("DraftAdministrativeData_DraftUUID") + if (!draftAdmin?.DraftAdministrativeData_DraftUUID) + return req.reject(500, "Failed to create draft") + + // Copy all attachments from the active source into the new draft + const sourceAttachments = await SELECT.from(Attachments).where({ + up__ID: sourceID, + }) + if (sourceAttachments.length > 0) { + const AttachmentsSrv = await cds.connect.to("attachments") + for (const att of sourceAttachments) { + await AttachmentsSrv.copy( + Attachments, + { ID: att.ID }, + Attachments.drafts, + { + up__ID: newDraft.ID, + DraftAdministrativeData_DraftUUID: + draftAdmin.DraftAdministrativeData_DraftUUID, + }, + ) + } + } + + return newDraft + } + async insertTestData() { const firstID = cds.utils.uuid() const secondID = cds.utils.uuid() From a9a17da6fdba22bbd80cb473a184dc050ea6fd47 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:19:35 +0200 Subject: [PATCH 03/17] Centralized URL creation --- lib/generic-handlers.js | 13 ++++++------- srv/basic.js | 35 ++++++++++++++++++++--------------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 9d312889..9975e395 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -57,10 +57,9 @@ async function finalizePrepareAttachment(data, req) { } if (!data.url) { - data.url = - isMultitenacyEnabled && objectStoreKind === "shared" - ? `${req.tenant}_${cds.utils.uuid()}` - : cds.utils.uuid() + const attachment = await cds.connect.to("attachments") + // Generate URL for object store + data.url = await attachment.createUrlForAttachment(data) } data.ID ??= cds.utils.uuid() @@ -367,11 +366,11 @@ async function validateAndInsertAttachmentFromDBHandler(data, target, req) { !(await validateAttachmentSize({ data, target, reject: req.reject }, true)) ) return - + + const attachment = await cds.connect.to("attachments") // Generate URL for object store - data.url = cds.utils.uuid() + data.url = await attachment.createUrlForAttachment(data) - const attachment = await cds.connect.to("attachments") // Let the attachments service handle storage (object store + DB metadata) await attachment.put(target, data) diff --git a/srv/basic.js b/srv/basic.js index 759ae065..8a87d8fd 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -557,6 +557,19 @@ class AttachmentsService extends cds.Service { return sanitized } + /** + * + * @param {*} data + * @returns + */ + createUrlForAttachment() { + const isMultiTenancyEnabled = !!cds.env.requires.multitenancy + const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind + return isMultiTenancyEnabled && objectStoreKind === "shared" + ? `${cds.context.tenant}_${cds.utils.uuid()}` + : cds.utils.uuid() + } + /** * Prepares a copy operation by validating the source and generating new identifiers. * Shared by all storage backends. @@ -565,7 +578,8 @@ class AttachmentsService extends cds.Service { * @returns {Promise<{ source: object, newID: string, newUrl: string }>} */ async _prepareCopy(sourceAttachments, sourceKeys) { - const source = await SELECT.one + // this.run so auth is enforced + const source = await this.run(SELECT.one .from(sourceAttachments, sourceKeys) .columns( "url", @@ -575,25 +589,21 @@ class AttachmentsService extends cds.Service { "hash", "status", "lastScan", - ) + )) if (!source) { const err = new Error("Source attachment not found") err.status = 404 throw err } - if (source.status === "Infected" || source.status === "Failed") { + if (source.status !== "Clean") { const err = new Error( - `Cannot copy attachment with status: ${source.status}`, + `Cannot copy attachment with status: ${source.status}. Only a Clean Status is allowed`, ) err.status = 400 throw err } - const isMultiTenancyEnabled = !!cds.env.requires.multitenancy - const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind - const newUrl = - isMultiTenancyEnabled && objectStoreKind === "shared" - ? `${cds.context.tenant}_${cds.utils.uuid()}` - : cds.utils.uuid() + const newUrl = this.createUrlForAttachment(source) + return { source, newID: cds.utils.uuid(), newUrl } } @@ -603,11 +613,6 @@ class AttachmentsService extends cds.Service { * Cloud backends override this to use native server-side copy. * Scan status, lastScan, and hash are inherited from the source — no re-scan needed. * - * Authorization note: this method uses raw CQL and does not enforce CDS - * service-level authorization (@requires / @restrict). Callers are responsible - * for verifying that the current user has read access to the source and write - * access to the target before invoking copy(). - * * Tenant note: only copies within the same tenant are supported. Cross-tenant * copies are not allowed because the storage backends resolve credentials for * the current tenant only. From 80a780a2b95028e7ff248dcadc62f2accec7a5c3 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:19:46 +0200 Subject: [PATCH 04/17] Formatting --- lib/generic-handlers.js | 3 +-- srv/basic.js | 36 +++++++++++++++++++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index 9975e395..ded188cc 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -366,12 +366,11 @@ async function validateAndInsertAttachmentFromDBHandler(data, target, req) { !(await validateAttachmentSize({ data, target, reject: req.reject }, true)) ) return - + const attachment = await cds.connect.to("attachments") // Generate URL for object store data.url = await attachment.createUrlForAttachment(data) - // Let the attachments service handle storage (object store + DB metadata) await attachment.put(target, data) } diff --git a/srv/basic.js b/srv/basic.js index 8a87d8fd..356a2940 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -558,16 +558,16 @@ class AttachmentsService extends cds.Service { } /** - * - * @param {*} data - * @returns + * + * @param {*} data + * @returns */ createUrlForAttachment() { const isMultiTenancyEnabled = !!cds.env.requires.multitenancy const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind return isMultiTenancyEnabled && objectStoreKind === "shared" - ? `${cds.context.tenant}_${cds.utils.uuid()}` - : cds.utils.uuid() + ? `${cds.context.tenant}_${cds.utils.uuid()}` + : cds.utils.uuid() } /** @@ -579,17 +579,19 @@ class AttachmentsService extends cds.Service { */ async _prepareCopy(sourceAttachments, sourceKeys) { // this.run so auth is enforced - const source = await this.run(SELECT.one - .from(sourceAttachments, sourceKeys) - .columns( - "url", - "filename", - "mimeType", - "note", - "hash", - "status", - "lastScan", - )) + const source = await this.run( + SELECT.one + .from(sourceAttachments, sourceKeys) + .columns( + "url", + "filename", + "mimeType", + "note", + "hash", + "status", + "lastScan", + ), + ) if (!source) { const err = new Error("Source attachment not found") err.status = 404 @@ -603,7 +605,7 @@ class AttachmentsService extends cds.Service { throw err } const newUrl = this.createUrlForAttachment(source) - + return { source, newID: cds.utils.uuid(), newUrl } } From 81e8ab9e78ce283cc9bd31b56b34d74fe9e9654e Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:45:53 +0200 Subject: [PATCH 05/17] Cleanup --- README.md | 47 +++++++++++---------------- eslint.config.mjs | 8 +++++ lib/generic-handlers.js | 3 -- srv/basic.js | 42 +++--------------------- tests/integration/attachments.test.js | 17 ---------- 5 files changed, 32 insertions(+), 85 deletions(-) diff --git a/README.md b/README.md index 3737680d..09159331 100755 --- a/README.md +++ b/README.md @@ -294,45 +294,30 @@ entity Incidents { The `AttachmentsService` exposes a programmatic `copy()` method that copies an attachment to a new record. On cloud storage backends (AWS S3, Azure Blob Storage, GCP Cloud Storage) this uses a backend-native server-side copy — no binary data is transferred through your application. On database storage it reads and inserts the content directly. -```js -const AttachmentsSrv = await cds.connect.to("attachments") - -await AttachmentsSrv.copy( - sourceAttachments, - { ID: sourceAttachmentID }, - targetAttachments, - { up__ID: targetParentID }, -) -``` - **Signature:** ```js -copy(sourceAttachments, sourceKeys, targetAttachments, (targetKeys = {})) +const AttachmentsSrv = await cds.connect.to("attachments") +await AttachmentsSrv.copy(sourceAttachmentsEntity, sourceKeys, targetAttachmentsEntity, (targetKeys = {})) ``` | Parameter | Description | | ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `sourceAttachments` | CDS entity definition of the source attachment composition. Pass `sourceAttachments.drafts` to copy from a draft-only source. | -| `sourceKeys` | Keys identifying the source attachment (e.g. `{ ID: '...' }`) | -| `targetAttachments` | CDS entity definition of the target attachment composition. Pass `targetAttachments.drafts` when the target parent is in an open draft editing session — the new record will be inserted into the draft shadow table and committed on save. | -| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID` (the draft session UUID of the target parent). | +| `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | +| `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | +| `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | +| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | -The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment with status `Infected` or `Failed` is rejected with a `400` error. +The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment when the status is not `Clean` is rejected with a `400` error. -> **Note:** Only copies within the same tenant are supported. Cross-tenant copies are not possible. - -> **Note:** `copy()` uses raw CQL internally and does not enforce CDS service-level authorization (`@requires` / `@restrict`). Callers are responsible for verifying that the current user has appropriate access to both the source and target entities before invoking `copy()`. +> [!NOTE] +> Only copies within the same tenant are supported. Cross-tenant copies are not possible. -**Supported scenarios:** +#### Examples -| Source | Target | Description | -| -------------------- | -------------------- | ------------------------------------------------------------------------------------------------------ | -| `Attachments` | `Attachments` | Copy between two active records, e.g. duplicating an incident. | -| `Attachments` | `Attachments.drafts` | Copy into a record that is in an open draft session, e.g. initialising a new incident from a template. | -| `Attachments.drafts` | `Attachments.drafts` | Copy from a draft-only source (e.g. a template that has never been activated) into another draft. | +
-**Example — copy between two active records:** +copy between two active records: ```js const { Incidents } = ProcessorService.entities @@ -345,7 +330,11 @@ await AttachmentsSrv.copy( ) ``` -**Example — copy into a new draft record (e.g. creating an incident from a template):** +
+ +
+ +copy into a new draft record (e.g. creating an incident from a template) ```js const { Incidents } = ProcessorService.entities @@ -367,6 +356,8 @@ await AttachmentsSrv.copy( ) ``` +
+ ### Non-Draft Upload For scenarios where the entity is not draft-enabled, for example [`tests/non-draft-request.http`](./tests/non-draft-request.http), separate HTTP requests for metadata creation and asset uploading need to be performed manually. diff --git a/eslint.config.mjs b/eslint.config.mjs index 0833481e..61f1db1b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -1,6 +1,14 @@ import cds from "@sap/cds/eslint.config.mjs" export default [ ...cds, + { + rules: { + "no-unused-vars": [ + 'error', + { argsIgnorePattern: '^_', reportUsedIgnorePattern: true }, + ] + } + }, { name: "test-files-config", files: ["tests/**/*"], diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index ded188cc..299bb333 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -9,9 +9,6 @@ const { } = require("./helper") const { getMime } = require("./mime") -const isMultitenacyEnabled = !!cds.env.requires.multitenancy -const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind - /** * Finalizes the preparation of a single attachment's data * @param {object} data - The attachment data diff --git a/srv/basic.js b/srv/basic.js index 356a2940..c57718f4 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -13,39 +13,6 @@ class AttachmentsService extends cds.Service { await this.delete(msg.data.url, msg.data.target) }) - this.on("CopyAttachment", async (msg) => { - const { - sourceTarget, - sourceKeys, - targetTarget, - targetKeys, - targetIsDraft = false, - } = msg.data - const sourceAttachments = cds.model.definitions[sourceTarget] - if (!sourceAttachments?.["@_is_media_data"]) { - const err = new Error( - `Invalid source entity: ${sourceTarget} is not an attachment entity`, - ) - err.status = 400 - throw err - } - const targetDef = cds.model.definitions[targetTarget] - if (!targetDef?.["@_is_media_data"]) { - const err = new Error( - `Invalid target entity: ${targetTarget} is not an attachment entity`, - ) - err.status = 400 - throw err - } - const targetAttachments = targetIsDraft ? targetDef?.drafts : targetDef - return this.copy( - sourceAttachments, - sourceKeys, - targetAttachments, - targetKeys, - ) - }) - this.on("DeleteInfectedAttachment", async (msg) => { const { target, hash, keys } = msg.data const attachment = await SELECT.one @@ -653,11 +620,12 @@ class AttachmentsService extends cds.Service { ...source, ID: newID, url: newUrl, - content, } - await INSERT(newRecord).into(targetAttachments) - const { content: _, ...metadata } = newRecord - return metadata + await INSERT(newRecord).into({ + ...newRecord, + content + }) + return newRecord } /** diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index 76c96de3..617e7fae 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -3494,23 +3494,6 @@ describe("Tests for copy() on AttachmentsService", () => { expect(newAtt.filename).toEqual("sample.pdf") expect(newAtt.mimeType).not.toEqual("application/x-evil") }) - - it("CopyAttachment event rejects non-attachment entity names", async () => { - const AttachmentsSrv = cds.unboxed(await cds.connect.to("attachments")) - const { ProcessorService } = cds.services - - // Use a real non-attachment entity name - const nonAttachmentEntity = ProcessorService.entities.Incidents.name - - await expect( - AttachmentsSrv.send("CopyAttachment", { - sourceTarget: nonAttachmentEntity, - sourceKeys: { ID: cds.utils.uuid() }, - targetTarget: nonAttachmentEntity, - targetKeys: {}, - }), - ).rejects.toMatchObject({ status: 400 }) - }) }) /** From 256cdc86afddf315826c94dab3aad041666bb0ab Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:46:12 +0200 Subject: [PATCH 06/17] Formatting --- README.md | 19 ++++++++++++------- eslint.config.mjs | 8 ++++---- srv/basic.js | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 09159331..27635299 100755 --- a/README.md +++ b/README.md @@ -298,15 +298,20 @@ The `AttachmentsService` exposes a programmatic `copy()` method that copies an a ```js const AttachmentsSrv = await cds.connect.to("attachments") -await AttachmentsSrv.copy(sourceAttachmentsEntity, sourceKeys, targetAttachmentsEntity, (targetKeys = {})) +await AttachmentsSrv.copy( + sourceAttachmentsEntity, + sourceKeys, + targetAttachmentsEntity, + (targetKeys = {}), +) ``` -| Parameter | Description | -| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | -| `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | -| `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | -| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | +| Parameter | Description | +| ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | +| `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | +| `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | +| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment when the status is not `Clean` is rejected with a `400` error. diff --git a/eslint.config.mjs b/eslint.config.mjs index 61f1db1b..bf8492b8 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -4,10 +4,10 @@ export default [ { rules: { "no-unused-vars": [ - 'error', - { argsIgnorePattern: '^_', reportUsedIgnorePattern: true }, - ] - } + "error", + { argsIgnorePattern: "^_", reportUsedIgnorePattern: true }, + ], + }, }, { name: "test-files-config", diff --git a/srv/basic.js b/srv/basic.js index c57718f4..d87c2bd4 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -623,7 +623,7 @@ class AttachmentsService extends cds.Service { } await INSERT(newRecord).into({ ...newRecord, - content + content, }) return newRecord } From 3b89a1c342dc159c2f55b4f7dfce75d223be7215 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:50:11 +0200 Subject: [PATCH 07/17] Update malwareScanner.js --- srv/malwareScanner.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index 396f43e5..9573472e 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -99,31 +99,31 @@ class MalwareScanner extends cds.ApplicationService { await this.updateStatus(_target, Object.assign({ hash }, keys), status) } - async getFileInformation(_target, keys) { + async getFileInformation(target, keys) { const dbResult = await SELECT.one - .from(_target.drafts || _target) + .from(target.drafts || target) .columns("mimeType") .where(keys) return dbResult } - async updateStatus(_target, keys, status) { - if (_target.drafts) { + async updateStatus(target, keys, status) { + if (target.drafts) { await Promise.all([ - UPDATE.entity(_target) + UPDATE.entity(target) .where(keys) .set({ status, lastScan: new Date() }), - UPDATE.entity(_target.drafts) + UPDATE.entity(target.drafts) .where(keys) .set({ status, lastScan: new Date() }), ]) } else { - await UPDATE.entity(_target) + await UPDATE.entity(target) .where(keys) .set({ status, lastScan: new Date() }) } LOG.info( - `Updated scan status to ${status} for ${_target.name}, ${JSON.stringify(keys)}`, + `Updated scan status to ${status} for ${target.name}, ${JSON.stringify(keys)}`, ) } From 00075f77ee003545e14910b6f0b148af0104d595 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:50:17 +0200 Subject: [PATCH 08/17] Update malwareScanner.js --- srv/malwareScanner.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index 9573472e..2c948d6a 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -110,9 +110,7 @@ class MalwareScanner extends cds.ApplicationService { async updateStatus(target, keys, status) { if (target.drafts) { await Promise.all([ - UPDATE.entity(target) - .where(keys) - .set({ status, lastScan: new Date() }), + UPDATE.entity(target).where(keys).set({ status, lastScan: new Date() }), UPDATE.entity(target.drafts) .where(keys) .set({ status, lastScan: new Date() }), From 538fb879880c819c04486414bc3f2b67b43d1e92 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:55:02 +0200 Subject: [PATCH 09/17] Invert protected fields to only allow up_ keys --- srv/basic.js | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/srv/basic.js b/srv/basic.js index d87c2bd4..1c59e8e4 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -485,27 +485,6 @@ class AttachmentsService extends cds.Service { } } - /** - * Fields that are never allowed in targetKeys — they are controlled by the - * copy logic and must not be overridden by callers. - */ - static _PROTECTED_FIELDS = new Set([ - "ID", - "url", - "content", - "hash", - "status", - "lastScan", - "filename", - "mimeType", - "note", - "statusNav", - "createdAt", - "createdBy", - "modifiedAt", - "modifiedBy", - ]) - /** * Strips protected fields from targetKeys so callers cannot override * security-sensitive metadata (status, hash, url, etc.). @@ -515,7 +494,7 @@ class AttachmentsService extends cds.Service { _sanitizeTargetKeys(targetKeys) { const sanitized = {} for (const [key, value] of Object.entries(targetKeys)) { - if (!AttachmentsService._PROTECTED_FIELDS.has(key)) { + if (key.startsWith('up_')) { sanitized[key] = value } else { LOG.warn(`Ignoring protected field in targetKeys: ${key}`) From b1ca23298bea191f2fff149ddefa925bc59ea9bd Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:55:08 +0200 Subject: [PATCH 10/17] Update basic.js --- srv/basic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srv/basic.js b/srv/basic.js index 1c59e8e4..59dfa6d6 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -494,7 +494,7 @@ class AttachmentsService extends cds.Service { _sanitizeTargetKeys(targetKeys) { const sanitized = {} for (const [key, value] of Object.entries(targetKeys)) { - if (key.startsWith('up_')) { + if (key.startsWith("up_")) { sanitized[key] = value } else { LOG.warn(`Ignoring protected field in targetKeys: ${key}`) From 7de5c3c2c81046c4a2049103cd0a0ae390058c07 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 14:59:57 +0200 Subject: [PATCH 11/17] Rename attachment parameter --- README.md | 2 +- srv/aws-s3.js | 12 ++++++------ srv/azure-blob-storage.js | 12 ++++++------ srv/basic.js | 26 +++++++++++++------------- srv/gcp.js | 12 ++++++------ tests/incidents-app/srv/services.js | 6 +++--- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 27635299..28730149 100755 --- a/README.md +++ b/README.md @@ -311,7 +311,7 @@ await AttachmentsSrv.copy( | `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | | `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | | `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | -| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachments` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | +| `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachmentsEntity` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment when the status is not `Clean` is rejected with a `400` error. diff --git a/srv/aws-s3.js b/srv/aws-s3.js index c4ab386d..93f5f88e 100644 --- a/srv/aws-s3.js +++ b/srv/aws-s3.js @@ -333,19 +333,19 @@ module.exports = class AWSAttachmentsService extends require("./object-store") { * @inheritdoc */ async copy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, - targetAttachments, + targetAttachmentsEntity, targetKeys = {}, ) { LOG.debug("Copying attachment (S3)", { - source: sourceAttachments.name, + source: sourceAttachmentsEntity.name, sourceKeys, - target: targetAttachments.name, + target: targetAttachmentsEntity.name, }) const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) const { source, newID, newUrl } = await this._prepareCopy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, ) const { client, bucket } = await this.retrieveClient() @@ -362,7 +362,7 @@ module.exports = class AWSAttachmentsService extends require("./object-store") { }), ) const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } - await INSERT(newRecord).into(targetAttachments) + await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord } diff --git a/srv/azure-blob-storage.js b/srv/azure-blob-storage.js index 63502136..23da68cd 100644 --- a/srv/azure-blob-storage.js +++ b/srv/azure-blob-storage.js @@ -309,19 +309,19 @@ module.exports = class AzureAttachmentsService extends ( * @inheritdoc */ async copy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, - targetAttachments, + targetAttachmentsEntity, targetKeys = {}, ) { LOG.debug("Copying attachment (Azure)", { - source: sourceAttachments.name, + source: sourceAttachmentsEntity.name, sourceKeys, - target: targetAttachments.name, + target: targetAttachmentsEntity.name, }) const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) const { source, newID, newUrl } = await this._prepareCopy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, ) const { containerClient } = await this.retrieveClient() @@ -334,7 +334,7 @@ module.exports = class AzureAttachmentsService extends ( const targetBlobClient = containerClient.getBlockBlobClient(newUrl) await targetBlobClient.syncCopyFromURL(sourceBlobClient.url) const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } - await INSERT(newRecord).into(targetAttachments) + await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord } diff --git a/srv/basic.js b/srv/basic.js index 59dfa6d6..aab2471f 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -519,15 +519,15 @@ class AttachmentsService extends cds.Service { /** * Prepares a copy operation by validating the source and generating new identifiers. * Shared by all storage backends. - * @param {import('@sap/cds').Entity} sourceAttachments - Source attachment entity definition + * @param {import('@sap/cds').Entity} sourceAttachmentsEntity - Source attachment entity definition * @param {object} sourceKeys - Keys identifying the source attachment (e.g. { ID: '...' }) * @returns {Promise<{ source: object, newID: string, newUrl: string }>} */ - async _prepareCopy(sourceAttachments, sourceKeys) { + async _prepareCopy(sourceAttachmentsEntity, sourceKeys) { // this.run so auth is enforced const source = await this.run( SELECT.one - .from(sourceAttachments, sourceKeys) + .from(sourceAttachmentsEntity, sourceKeys) .columns( "url", "filename", @@ -565,11 +565,11 @@ class AttachmentsService extends cds.Service { * copies are not allowed because the storage backends resolve credentials for * the current tenant only. * - * @param {import('@sap/cds').Entity} sourceAttachments - Source attachment entity definition. - * Pass `sourceAttachments.drafts` to copy from a draft-only source. + * @param {import('@sap/cds').Entity} sourceAttachmentsEntity - Source attachment entity definition. + * Pass `sourceAttachmentsEntity.drafts` to copy from a draft-only source. * @param {object} sourceKeys - Keys identifying the source attachment (e.g. { ID: '...' }) - * @param {import('@sap/cds').Entity} targetAttachments - Target attachment entity definition. - * Pass `targetAttachments.drafts` to insert into the draft shadow table (i.e. when the target + * @param {import('@sap/cds').Entity} targetAttachmentsEntity - Target attachment entity definition. + * Pass `targetAttachmentsEntity.drafts` to insert into the draft shadow table (i.e. when the target * parent entity is currently in a draft editing session). In that case targetKeys must include * DraftAdministrativeData_DraftUUID. * @param {object} [targetKeys={}] - Parent FK fields for the new record (e.g. { up__ID: '...' }). @@ -578,22 +578,22 @@ class AttachmentsService extends cds.Service { * @returns {Promise} - New attachment metadata (without content) */ async copy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, - targetAttachments, + targetAttachmentsEntity, targetKeys = {}, ) { LOG.debug("Copying attachment (DB)", { - source: sourceAttachments.name, + source: sourceAttachmentsEntity.name, sourceKeys, - target: targetAttachments.name, + target: targetAttachmentsEntity.name, }) const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) const { source, newID, newUrl } = await this._prepareCopy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, ) - const content = await this.get(sourceAttachments, sourceKeys) + const content = await this.get(sourceAttachmentsEntity, sourceKeys) const newRecord = { ...safeTargetKeys, ...source, diff --git a/srv/gcp.js b/srv/gcp.js index 5a56de31..08d990fc 100644 --- a/srv/gcp.js +++ b/srv/gcp.js @@ -350,19 +350,19 @@ module.exports = class GoogleAttachmentsService extends ( * @inheritdoc */ async copy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, - targetAttachments, + targetAttachmentsEntity, targetKeys = {}, ) { LOG.debug("Copying attachment (GCP)", { - source: sourceAttachments.name, + source: sourceAttachmentsEntity.name, sourceKeys, - target: targetAttachments.name, + target: targetAttachmentsEntity.name, }) const safeTargetKeys = this._sanitizeTargetKeys(targetKeys) const { source, newID, newUrl } = await this._prepareCopy( - sourceAttachments, + sourceAttachmentsEntity, sourceKeys, ) const { bucket } = await this.retrieveClient() @@ -373,7 +373,7 @@ module.exports = class GoogleAttachmentsService extends ( } await bucket.file(source.url).copy(bucket.file(newUrl)) const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } - await INSERT(newRecord).into(targetAttachments) + await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord } diff --git a/tests/incidents-app/srv/services.js b/tests/incidents-app/srv/services.js index 1038f2a5..780a6408 100644 --- a/tests/incidents-app/srv/services.js +++ b/tests/incidents-app/srv/services.js @@ -55,12 +55,12 @@ class ProcessorService extends cds.ApplicationService { return req.reject(500, "Failed to create draft") // Copy all attachments from the active source into the new draft - const sourceAttachments = await SELECT.from(Attachments).where({ + const sourceAttachmentsEntity = await SELECT.from(Attachments).where({ up__ID: sourceID, }) - if (sourceAttachments.length > 0) { + if (sourceAttachmentsEntity.length > 0) { const AttachmentsSrv = await cds.connect.to("attachments") - for (const att of sourceAttachments) { + for (const att of sourceAttachmentsEntity) { await AttachmentsSrv.copy( Attachments, { ID: att.ID }, From b59b5a6a976ddc3c88fcf9acbacb83170d41dbdf Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 15:00:03 +0200 Subject: [PATCH 12/17] Update README.md --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 28730149..1b2f8a89 100755 --- a/README.md +++ b/README.md @@ -306,11 +306,11 @@ await AttachmentsSrv.copy( ) ``` -| Parameter | Description | -| ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | -| `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | -| `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | +| Parameter | Description | +| ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `sourceAttachmentsEntity` | CDS entity definition of the source attachment composition. | +| `sourceKeys` | Keys of the attachment (e.g. `{ ID: '...' }`) | +| `targetAttachmentsEntity` | CDS entity definition of the target attachment composition. | | `targetKeys` | Parent FK fields for the new record (e.g. `{ up__ID: '...' }`). When `targetAttachmentsEntity` is a draft table, must also include `DraftAdministrativeData_DraftUUID`. | The scan `status`, `lastScan`, and `hash` are inherited from the source — no re-scan is triggered since the binary content is identical. Copying an attachment when the status is not `Clean` is rejected with a `400` error. From ac46ce76288bf3a1e064c669021e422eaa1b8f64 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 15:01:41 +0200 Subject: [PATCH 13/17] Update basic.js --- srv/basic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srv/basic.js b/srv/basic.js index aab2471f..a14da7cc 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -494,7 +494,7 @@ class AttachmentsService extends cds.Service { _sanitizeTargetKeys(targetKeys) { const sanitized = {} for (const [key, value] of Object.entries(targetKeys)) { - if (key.startsWith("up_")) { + if (key.startsWith("up_") || key.startsWith('DraftAdministrativeData')) { sanitized[key] = value } else { LOG.warn(`Ignoring protected field in targetKeys: ${key}`) From e4bd3bfa5450eb3eeba26e64b345d9932255894b Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 15:02:04 +0200 Subject: [PATCH 14/17] Update basic.js --- srv/basic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srv/basic.js b/srv/basic.js index a14da7cc..57a8e71c 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -494,7 +494,7 @@ class AttachmentsService extends cds.Service { _sanitizeTargetKeys(targetKeys) { const sanitized = {} for (const [key, value] of Object.entries(targetKeys)) { - if (key.startsWith("up_") || key.startsWith('DraftAdministrativeData')) { + if (key.startsWith("up_") || key.startsWith("DraftAdministrativeData")) { sanitized[key] = value } else { LOG.warn(`Ignoring protected field in targetKeys: ${key}`) From 3d0c446cb8b08c20363b6f2a495f3195d3ece343 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 15:32:28 +0200 Subject: [PATCH 15/17] Fixes --- srv/basic.js | 14 ++++++++------ tests/integration/attachments.test.js | 28 ++++++++++++++------------- tests/utils/testUtils.js | 10 ++++++++++ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/srv/basic.js b/srv/basic.js index 57a8e71c..8902d2fb 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -524,10 +524,11 @@ class AttachmentsService extends cds.Service { * @returns {Promise<{ source: object, newID: string, newUrl: string }>} */ async _prepareCopy(sourceAttachmentsEntity, sourceKeys) { - // this.run so auth is enforced - const source = await this.run( - SELECT.one - .from(sourceAttachmentsEntity, sourceKeys) + // srv.run so auth is enforced + const srv = await cds.connect.to(sourceAttachmentsEntity._service?.name ?? 'db') + const source = await srv.run( + SELECT + .one.from(sourceAttachmentsEntity, sourceKeys) .columns( "url", "filename", @@ -595,12 +596,13 @@ class AttachmentsService extends cds.Service { ) const content = await this.get(sourceAttachmentsEntity, sourceKeys) const newRecord = { - ...safeTargetKeys, ...source, + // Must be spread into afterwards else source up_ overrides target keys + ...safeTargetKeys, ID: newID, url: newUrl, } - await INSERT(newRecord).into({ + await INSERT.into(targetAttachmentsEntity).entries({ ...newRecord, content, }) diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index d94ef84e..6f0bac5e 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -6,6 +6,7 @@ const { delay, waitForMalwareDeletion, waitForDeletion, + runWithUser, } = require("../utils/testUtils") const { createReadStream, readFileSync } = cds.utils.fs const { join, basename } = cds.utils.path @@ -14,6 +15,7 @@ const { Readable } = require("stream") const app = join(__dirname, "../incidents-app") const { axios, GET, POST, DELETE, PATCH, PUT } = cds.test(app) axios.defaults.auth = { username: "alice" } +const alice = new cds.User({ id: "alice", roles: { admin: 1, support: 1 } }) let utils = null @@ -3295,12 +3297,12 @@ describe("Tests for copy() on AttachmentsService", () => { const { ProcessorService } = cds.services const Attachments = ProcessorService.entities["Incidents.attachments"] - const newAtt = await AttachmentsSrv.copy( + const newAtt = await runWithUser(alice, () => AttachmentsSrv.copy( Attachments, { ID: sourceAttachmentID }, Attachments, { up__ID: targetIncidentID }, - ) + )) expect(newAtt.ID).not.toEqual(sourceAttachmentID) expect(newAtt.url).toBeTruthy() expect(newAtt.filename).toEqual("sample.pdf") @@ -3352,7 +3354,7 @@ describe("Tests for copy() on AttachmentsService", () => { .columns("DraftAdministrativeData_DraftUUID") expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() - const newAtt = await AttachmentsSrv.copy( + const newAtt = await await runWithUser(alice, () => AttachmentsSrv.copy( Attachments, { ID: sourceAttachmentID }, Attachments.drafts, @@ -3361,7 +3363,7 @@ describe("Tests for copy() on AttachmentsService", () => { DraftAdministrativeData_DraftUUID: targetDraft.DraftAdministrativeData_DraftUUID, }, - ) + )) expect(newAtt.ID).toBeTruthy() expect(newAtt.status).toEqual("Clean") @@ -3423,7 +3425,7 @@ describe("Tests for copy() on AttachmentsService", () => { expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() // Source is the active Attachments entity (uploaded via draft, now active after save) - const newAtt = await AttachmentsSrv.copy( + const newAtt = await await runWithUser(alice, () => AttachmentsSrv.copy( Attachments, { ID: sourceAttachmentID }, Attachments.drafts, @@ -3432,7 +3434,7 @@ describe("Tests for copy() on AttachmentsService", () => { DraftAdministrativeData_DraftUUID: targetDraft.DraftAdministrativeData_DraftUUID, }, - ) + )) expect(newAtt.ID).toBeTruthy() expect(newAtt.status).toEqual("Clean") @@ -3465,9 +3467,9 @@ describe("Tests for copy() on AttachmentsService", () => { ) await expect( - AttachmentsSrv.copy(Attachments, { ID: infectedID }, Attachments, { + runWithUser(alice, () => AttachmentsSrv.copy(Attachments, { ID: infectedID }, Attachments, { up__ID: incidentID, - }), + })), ).rejects.toMatchObject({ status: 400 }) }) @@ -3478,9 +3480,9 @@ describe("Tests for copy() on AttachmentsService", () => { const Attachments = ProcessorService.entities["Incidents.attachments"] await expect( - AttachmentsSrv.copy(Attachments, { ID: cds.utils.uuid() }, Attachments, { + runWithUser(alice, () => AttachmentsSrv.copy(Attachments, { ID: cds.utils.uuid() }, Attachments, { up__ID: incidentID, - }), + })), ).rejects.toMatchObject({ status: 404 }) }) @@ -3509,7 +3511,7 @@ describe("Tests for copy() on AttachmentsService", () => { const Attachments = ProcessorService.entities["Incidents.attachments"] // Attempt to override protected fields via targetKeys - const newAtt = await AttachmentsSrv.copy( + const newAtt = await runWithUser(alice, () => AttachmentsSrv.copy( Attachments, { ID: sourceAttachmentID }, Attachments, @@ -3520,7 +3522,7 @@ describe("Tests for copy() on AttachmentsService", () => { filename: "evil.exe", mimeType: "application/x-evil", }, - ) + )) // Protected fields must reflect the source, not the attacker's values expect(newAtt.status).toEqual("Clean") @@ -3595,4 +3597,4 @@ async function uploadDraftAttachment( `odata/v4/processor/Incidents(ID=${incidentId},IsActiveEntity=true)/${entityName}`, ) return response.data.value[0]?.ID -} +} \ No newline at end of file diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 3cf38f5a..1f59b0ef 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -133,10 +133,20 @@ async function newIncident( } } +async function runWithUser(user, fn) { + const ctx = cds.EventContext.for({ + id: cds.utils.uuid(), + http: { req: null, res: null }, + }) + ctx.user = user + return cds._with(ctx, fn) +} + module.exports = { delay, waitForScanStatus, newIncident, waitForDeletion, waitForMalwareDeletion, + runWithUser } From 01176fc5aa7195253fce7acfacf05b4eb2da474d Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 15:32:43 +0200 Subject: [PATCH 16/17] Formatting --- srv/basic.js | 8 +- tests/integration/attachments.test.js | 107 +++++++++++++++----------- tests/utils/testUtils.js | 2 +- 3 files changed, 68 insertions(+), 49 deletions(-) diff --git a/srv/basic.js b/srv/basic.js index 8902d2fb..83598c48 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -525,10 +525,12 @@ class AttachmentsService extends cds.Service { */ async _prepareCopy(sourceAttachmentsEntity, sourceKeys) { // srv.run so auth is enforced - const srv = await cds.connect.to(sourceAttachmentsEntity._service?.name ?? 'db') + const srv = await cds.connect.to( + sourceAttachmentsEntity._service?.name ?? "db", + ) const source = await srv.run( - SELECT - .one.from(sourceAttachmentsEntity, sourceKeys) + SELECT.one + .from(sourceAttachmentsEntity, sourceKeys) .columns( "url", "filename", diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index 6f0bac5e..24d3cb18 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -3297,12 +3297,14 @@ describe("Tests for copy() on AttachmentsService", () => { const { ProcessorService } = cds.services const Attachments = ProcessorService.entities["Incidents.attachments"] - const newAtt = await runWithUser(alice, () => AttachmentsSrv.copy( - Attachments, - { ID: sourceAttachmentID }, - Attachments, - { up__ID: targetIncidentID }, - )) + const newAtt = await runWithUser(alice, () => + AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments, + { up__ID: targetIncidentID }, + ), + ) expect(newAtt.ID).not.toEqual(sourceAttachmentID) expect(newAtt.url).toBeTruthy() expect(newAtt.filename).toEqual("sample.pdf") @@ -3354,16 +3356,18 @@ describe("Tests for copy() on AttachmentsService", () => { .columns("DraftAdministrativeData_DraftUUID") expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() - const newAtt = await await runWithUser(alice, () => AttachmentsSrv.copy( - Attachments, - { ID: sourceAttachmentID }, - Attachments.drafts, - { - up__ID: targetIncidentID, - DraftAdministrativeData_DraftUUID: - targetDraft.DraftAdministrativeData_DraftUUID, - }, - )) + const newAtt = await await runWithUser(alice, () => + AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments.drafts, + { + up__ID: targetIncidentID, + DraftAdministrativeData_DraftUUID: + targetDraft.DraftAdministrativeData_DraftUUID, + }, + ), + ) expect(newAtt.ID).toBeTruthy() expect(newAtt.status).toEqual("Clean") @@ -3425,16 +3429,18 @@ describe("Tests for copy() on AttachmentsService", () => { expect(targetDraft?.DraftAdministrativeData_DraftUUID).toBeTruthy() // Source is the active Attachments entity (uploaded via draft, now active after save) - const newAtt = await await runWithUser(alice, () => AttachmentsSrv.copy( - Attachments, - { ID: sourceAttachmentID }, - Attachments.drafts, - { - up__ID: targetIncidentID, - DraftAdministrativeData_DraftUUID: - targetDraft.DraftAdministrativeData_DraftUUID, - }, - )) + const newAtt = await await runWithUser(alice, () => + AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments.drafts, + { + up__ID: targetIncidentID, + DraftAdministrativeData_DraftUUID: + targetDraft.DraftAdministrativeData_DraftUUID, + }, + ), + ) expect(newAtt.ID).toBeTruthy() expect(newAtt.status).toEqual("Clean") @@ -3467,9 +3473,11 @@ describe("Tests for copy() on AttachmentsService", () => { ) await expect( - runWithUser(alice, () => AttachmentsSrv.copy(Attachments, { ID: infectedID }, Attachments, { - up__ID: incidentID, - })), + runWithUser(alice, () => + AttachmentsSrv.copy(Attachments, { ID: infectedID }, Attachments, { + up__ID: incidentID, + }), + ), ).rejects.toMatchObject({ status: 400 }) }) @@ -3480,9 +3488,16 @@ describe("Tests for copy() on AttachmentsService", () => { const Attachments = ProcessorService.entities["Incidents.attachments"] await expect( - runWithUser(alice, () => AttachmentsSrv.copy(Attachments, { ID: cds.utils.uuid() }, Attachments, { - up__ID: incidentID, - })), + runWithUser(alice, () => + AttachmentsSrv.copy( + Attachments, + { ID: cds.utils.uuid() }, + Attachments, + { + up__ID: incidentID, + }, + ), + ), ).rejects.toMatchObject({ status: 404 }) }) @@ -3511,18 +3526,20 @@ describe("Tests for copy() on AttachmentsService", () => { const Attachments = ProcessorService.entities["Incidents.attachments"] // Attempt to override protected fields via targetKeys - const newAtt = await runWithUser(alice, () => AttachmentsSrv.copy( - Attachments, - { ID: sourceAttachmentID }, - Attachments, - { - up__ID: targetIncidentID, - status: "Unscanned", - hash: "tampered-hash", - filename: "evil.exe", - mimeType: "application/x-evil", - }, - )) + const newAtt = await runWithUser(alice, () => + AttachmentsSrv.copy( + Attachments, + { ID: sourceAttachmentID }, + Attachments, + { + up__ID: targetIncidentID, + status: "Unscanned", + hash: "tampered-hash", + filename: "evil.exe", + mimeType: "application/x-evil", + }, + ), + ) // Protected fields must reflect the source, not the attacker's values expect(newAtt.status).toEqual("Clean") @@ -3597,4 +3614,4 @@ async function uploadDraftAttachment( `odata/v4/processor/Incidents(ID=${incidentId},IsActiveEntity=true)/${entityName}`, ) return response.data.value[0]?.ID -} \ No newline at end of file +} diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 1f59b0ef..81e71c2f 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -148,5 +148,5 @@ module.exports = { newIncident, waitForDeletion, waitForMalwareDeletion, - runWithUser + runWithUser, } From 2d1d348d0d1b9200f61c691d9663d3beef0a7e93 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Mon, 30 Mar 2026 16:14:08 +0200 Subject: [PATCH 17/17] Fix --- srv/aws-s3.js | 2 +- srv/azure-blob-storage.js | 2 +- srv/gcp.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/srv/aws-s3.js b/srv/aws-s3.js index 93f5f88e..f0e02a13 100644 --- a/srv/aws-s3.js +++ b/srv/aws-s3.js @@ -361,7 +361,7 @@ module.exports = class AWSAttachmentsService extends require("./object-store") { Key: newUrl, }), ) - const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + const newRecord = { ...source, ...safeTargetKeys, ID: newID, url: newUrl } await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord } diff --git a/srv/azure-blob-storage.js b/srv/azure-blob-storage.js index 23da68cd..c312bc6a 100644 --- a/srv/azure-blob-storage.js +++ b/srv/azure-blob-storage.js @@ -333,7 +333,7 @@ module.exports = class AzureAttachmentsService extends ( const sourceBlobClient = containerClient.getBlockBlobClient(source.url) const targetBlobClient = containerClient.getBlockBlobClient(newUrl) await targetBlobClient.syncCopyFromURL(sourceBlobClient.url) - const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + const newRecord = { ...source, ...safeTargetKeys, ID: newID, url: newUrl } await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord } diff --git a/srv/gcp.js b/srv/gcp.js index 08d990fc..fa92954d 100644 --- a/srv/gcp.js +++ b/srv/gcp.js @@ -372,7 +372,7 @@ module.exports = class GoogleAttachmentsService extends ( throw err } await bucket.file(source.url).copy(bucket.file(newUrl)) - const newRecord = { ...safeTargetKeys, ...source, ID: newID, url: newUrl } + const newRecord = { ...source, ...safeTargetKeys, ID: newID, url: newUrl } await INSERT(newRecord).into(targetAttachmentsEntity) return newRecord }