From fc158d3b0d8407a26b622be816120cc3ee97bd21 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Mon, 16 Mar 2026 15:24:02 +0100 Subject: [PATCH 01/26] feat: handling malware scan concurrency limitation --- srv/malwareScanner.js | 193 +++++++++++++------ tests/unit/malwareScanner.test.js | 296 ++++++++++++++++++++++++++++++ 2 files changed, 429 insertions(+), 60 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index 396f43e5..a44663e7 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -1,9 +1,35 @@ const cds = require("@sap/cds") const crypto = require("crypto") const https = require("https") +const axios = require("axios") const { URL } = require("url") const LOG = cds.log("attachments") +class Semaphore { + constructor(max) { + this._max = max + this._current = 0 + this._queue = [] + } + + acquire() { + if (this._current < this._max) { + this._current++ + return Promise.resolve() + } + return new Promise((resolve) => this._queue.push(resolve)) + } + + release() { + if (this._queue.length > 0) { + const next = this._queue.shift() + next() + } else { + this._current-- + } + } +} + class MalwareScanner extends cds.ApplicationService { get credentials() { return getCredentials() @@ -13,6 +39,16 @@ class MalwareScanner extends cds.ApplicationService { this.on("ScanAttachmentsFile", this.scanAttachmentsFile) this.on("scan", this.scanFile) + const config = cds.env.requires?.malwareScanner + this.retryConfig = { + enabled: config?.retry?.enabled ?? true, + maxAttempts: config?.retry?.maxAttempts ?? 5, + initialDelay: config?.retry?.initialDelay ?? 1000, + maxDelay: config?.retry?.maxDelay ?? 30000, + } + const maxConcurrent = config?.maxConcurrentScans ?? 5 + this.semaphore = maxConcurrent > 0 ? new Semaphore(maxConcurrent) : null + return super.init() } @@ -24,6 +60,15 @@ class MalwareScanner extends cds.ApplicationService { * @param {{data: {target: string, keys: object}}} msg The target is the CSN Entity name, which is used to lookup entity via cds.model.definitions[]. */ async scanAttachmentsFile(msg) { + if (this.semaphore) await this.semaphore.acquire() + try { + return await this._scanAttachmentsFile(msg) + } finally { + if (this.semaphore) this.semaphore.release() + } + } + + async _scanAttachmentsFile(msg) { const { target, keys } = msg.data const scanEnabled = cds.env.requires?.attachments?.scan ?? true if (!scanEnabled) { @@ -130,23 +175,15 @@ class MalwareScanner extends cds.ApplicationService { /** * Scans the passed over file * @param {*} req - The request object - * @param {string} fileName - The name of the file being scanned */ async scanFile(req) { const { file } = req.data - let response const scanStartTime = Date.now() try { - // Prepare request options const url = new URL(`https://${this.credentials.uri}/scan`) - const requestOptions = { - method: "POST", - hostname: url.hostname, - port: url.port || 443, - path: url.pathname, - headers: {}, - } + const headers = {} + let httpsAgent if (this.credentials?.certificate && this.credentials?.key) { LOG.debug("Using mTLS authentication for malware scanning") @@ -169,17 +206,18 @@ class MalwareScanner extends cds.ApplicationService { }) } - requestOptions.cert = this.credentials.certificate - requestOptions.key = this.credentials.key - requestOptions.rejectUnauthorized = false + httpsAgent = new https.Agent({ + cert: this.credentials.certificate, + key: this.credentials.key, + rejectUnauthorized: false, + }) LOG.debug("Using mTLS authorization") } else if (this.credentials?.username && this.credentials?.password) { - // Basic Auth: set Authorization header LOG.warn( "Deprecated: Basic Authentication for malware scanning is deprecated and will be removed in future releases.", ) - requestOptions.headers.Authorization = + headers.Authorization = "Basic " + Buffer.from( `${this.credentials.username}:${this.credentials.password}`, @@ -192,30 +230,36 @@ class MalwareScanner extends cds.ApplicationService { ) } - response = await new Promise((resolve, reject) => { - const req = https.request(requestOptions, (res) => { - let data = "" - res.on("data", (chunk) => (data += chunk)) - res.on("end", () => { - resolve({ - status: res.statusCode, - ok: res.statusCode >= 200 && res.statusCode < 300, - data, - }) - }) - }) - req.on("error", reject) - - file.pipe(req) - }) - - if (!response.ok) { - const json = JSON.parse(response.data || "{}") - const errorMsg = - JSON.stringify(json) || - response.statusText || - "Unknown error from malware scanner" - return req.reject(response.status, `Scanning failed: ${errorMsg}`) + // Buffer the stream so retries can re-send the same data + const chunks = [] + for await (const chunk of file) { + chunks.push(chunk) + } + const buffer = Buffer.concat(chunks) + + const response = await this._scanWithRetry(url, buffer, headers, httpsAgent) + + /** + * @typedef {Object} MalwareScanResponse + * @property {boolean} malwareDetected - Indicates whether the scan engine detected a threat. + * @property {boolean} encryptedContentDetected - Indicates whether the file has encrypted parts, which could not be scanned. + * @property {number} scanSize - Size in bytes of the scanned file. + * @property {string} finding - This field may contain information about detected malware. + * @property {string} mimeType - Indicates the detected MIME type for the scanned file. + * @property {string} SHA256 - SHA-256 hash of the scanned file. + */ + /** @type {MalwareScanResponse} */ + const responseJson = response.data + const scanDuration = Date.now() - scanStartTime + LOG.debug(`Malware scan response`, { scanDuration, response: responseJson }) + + return { + isMalware: responseJson.malwareDetected, + encryptedContentDetected: responseJson.encryptedContentDetected, + scanSize: responseJson.scanSize, + finding: responseJson.finding, + mimeType: responseJson.mimeType, + hash: responseJson.SHA256, } } catch (error) { const scanDuration = Date.now() - scanStartTime @@ -230,28 +274,57 @@ class MalwareScanner extends cds.ApplicationService { } finally { file?.destroy() } + } - /** - * @typedef {Object} MalwareScanResponse - * @property {boolean} malwareDetected - Indicates whether the scan engine detected a threat. - * @property {boolean} encryptedContentDetected - Indicates whether the file has encrypted parts, which could not be scanned. - * @property {number} scanSize - Size in bytes of the scanned file. Use the file size to validate the success of data transmission. - * @property {string} finding - This field may contain information about detected malware. - * @property {string} mimeType - Indicates the detected MIME type for the scanned file. This data may not be reliable and results may vary on different service providers. - * @property {string} SHA256 - SHA-256 hash of the scanned file. Use the hash to validate the success of data transmission. - */ - /** @type {MalwareScanResponse} */ - const responseJson = JSON.parse(response.data) - const scanDuration = Date.now() - scanStartTime - LOG.debug(`Malware scan response`, { scanDuration, response: responseJson }) - - return { - isMalware: responseJson.malwareDetected, - encryptedContentDetected: responseJson.encryptedContentDetected, - scanSize: responseJson.scanSize, - finding: responseJson.finding, - mimeType: responseJson.mimeType, - hash: responseJson.SHA256, + async _scanWithRetry(url, buffer, headers, httpsAgent) { + const { enabled, maxAttempts, initialDelay, maxDelay } = this.retryConfig + const attempts = enabled ? maxAttempts : 1 + + for (let attempt = 1; attempt <= attempts; attempt++) { + try { + const response = await axios.post(url.toString(), buffer, { + headers, + httpsAgent, + // Axios throws on non-2xx by default; we need to handle 429 ourselves + validateStatus: (status) => status >= 200 && status < 300, + }) + if (attempt > 1) { + LOG.info(`Malware scan succeeded after ${attempt} attempts`) + } + return response + } catch (err) { + const status = err.response?.status + if (status !== 429 || attempt === attempts) { + if (status === 429) { + throw new Error( + `Malware scan failed after ${attempts} attempts (last status: 429)`, + ) + } + // Non-429 error or non-Axios error — rethrow as-is + const data = err.response?.data + if (data) { + const errorMsg = + JSON.stringify(data) || "Unknown error from malware scanner" + throw new Error(`Scanning failed: ${errorMsg}`) + } + throw err + } + + // Calculate delay for retry + let delay + const retryAfter = err.response?.headers?.["retry-after"] + if (retryAfter) { + delay = Math.min(parseInt(retryAfter, 10) * 1000, maxDelay) + } else { + const jitter = Math.random() * 500 + delay = Math.min(initialDelay * Math.pow(2, attempt - 1) + jitter, maxDelay) + } + + LOG.debug( + `Malware scan received 429, retrying in ${Math.round(delay)}ms (attempt ${attempt}/${attempts})`, + ) + await new Promise((resolve) => setTimeout(resolve, delay)) + } } } } diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index b1bafc5e..7642eed2 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -3,6 +3,9 @@ const cds = require("@sap/cds") const { join } = cds.utils.path cds.test(join(__dirname, "../incidents-app")) +const axios = require("axios") +jest.mock("axios") + const MalwareScanner = require("../../srv/malwareScanner") let scanner @@ -225,3 +228,296 @@ describe("getCredentials", () => { ) }) }) + +// --------------------------------------------------------------------------- +// _scanWithRetry — retry on 429 +// --------------------------------------------------------------------------- + +describe("_scanWithRetry (retry on 429)", () => { + beforeEach(() => { + // Fast retries for tests + scanner.retryConfig = { + enabled: true, + maxAttempts: 3, + initialDelay: 1, + maxDelay: 10, + } + }) + + it("returns response on first success (no retry needed)", async () => { + const successResponse = { status: 200, data: { malwareDetected: false } } + axios.post.mockResolvedValueOnce(successResponse) + + const result = await scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ) + expect(result).toBe(successResponse) + expect(axios.post).toHaveBeenCalledTimes(1) + }) + + it("retries on 429 then succeeds", async () => { + const err429 = new Error("429") + err429.response = { status: 429, headers: {} } + const successResponse = { status: 200, data: { malwareDetected: false } } + + axios.post + .mockRejectedValueOnce(err429) + .mockResolvedValueOnce(successResponse) + + const result = await scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ) + expect(result).toBe(successResponse) + expect(axios.post).toHaveBeenCalledTimes(2) + }) + + it("throws after exhausting all retry attempts on 429", async () => { + const err429 = new Error("429") + err429.response = { status: 429, headers: {} } + + axios.post + .mockRejectedValueOnce(err429) + .mockRejectedValueOnce(err429) + .mockRejectedValueOnce(err429) + + await expect( + scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ), + ).rejects.toThrow("Malware scan failed after 3 attempts (last status: 429)") + expect(axios.post).toHaveBeenCalledTimes(3) + }) + + it("respects Retry-After header", async () => { + const err429 = new Error("429") + err429.response = { status: 429, headers: { "retry-after": "1" } } + const successResponse = { status: 200, data: { malwareDetected: false } } + + axios.post + .mockRejectedValueOnce(err429) + .mockResolvedValueOnce(successResponse) + + const start = Date.now() + await scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ) + // Retry-After: 1 second = 1000ms, but capped at maxDelay (10ms in test config) + // So the delay should be ~10ms, not 1000ms + const elapsed = Date.now() - start + expect(elapsed).toBeLessThan(500) + }) + + it("does not retry when retry is disabled", async () => { + scanner.retryConfig.enabled = false + + const err429 = new Error("429") + err429.response = { status: 429, headers: {} } + + axios.post.mockRejectedValueOnce(err429) + + await expect( + scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ), + ).rejects.toThrow("Malware scan failed after 1 attempts (last status: 429)") + expect(axios.post).toHaveBeenCalledTimes(1) + }) + + it("does not retry on non-429 errors", async () => { + const err500 = new Error("500") + err500.response = { status: 500, data: { error: "Internal Server Error" } } + + axios.post.mockRejectedValueOnce(err500) + + await expect( + scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ), + ).rejects.toThrow("Scanning failed:") + expect(axios.post).toHaveBeenCalledTimes(1) + }) + + it("rethrows network errors without retry", async () => { + const networkErr = new Error("ECONNREFUSED") + + axios.post.mockRejectedValueOnce(networkErr) + + await expect( + scanner._scanWithRetry( + new URL("https://host/scan"), + Buffer.from("data"), + {}, + undefined, + ), + ).rejects.toThrow("ECONNREFUSED") + expect(axios.post).toHaveBeenCalledTimes(1) + }) +}) + +// --------------------------------------------------------------------------- +// Semaphore / concurrency limiting +// --------------------------------------------------------------------------- + +describe("concurrency limiting", () => { + const target = "ProcessorService.Incidents.attachments" + const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } + + beforeEach(() => { + // Ensure attachmentsSrv.get returns a truthy value so scan flow proceeds to this.scan() + attachmentsSvc.get.mockResolvedValue({}) + }) + + it("limits concurrent scans to maxConcurrentScans", async () => { + scanner.semaphore = { _max: 2, _current: 0, _queue: [], + acquire() { + if (this._current < this._max) { this._current++; return Promise.resolve() } + return new Promise((resolve) => this._queue.push(resolve)) + }, + release() { + if (this._queue.length > 0) { this._queue.shift()() } else { this._current-- } + }, + } + scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } + + let activeConcurrent = 0 + let peakConcurrent = 0 + const resolvers = [] + + scanner.scan = jest.fn(() => { + activeConcurrent++ + peakConcurrent = Math.max(peakConcurrent, activeConcurrent) + return new Promise((resolve) => { + resolvers.push(() => { + activeConcurrent-- + resolve({ isMalware: false, hash: "h" }) + }) + }) + }) + jest.spyOn(scanner, "updateStatus").mockResolvedValue() + + const promises = [] + for (let i = 0; i < 5; i++) { + promises.push( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ) + } + + // Wait for first batch to start + await new Promise((r) => setTimeout(r, 50)) + expect(activeConcurrent).toBe(2) + expect(resolvers).toHaveLength(2) + + // Complete first two scans + resolvers.shift()() + resolvers.shift()() + await new Promise((r) => setTimeout(r, 50)) + + // Next batch should have started + expect(activeConcurrent).toBe(2) + + // Complete remaining + while (resolvers.length) resolvers.shift()() + await new Promise((r) => setTimeout(r, 50)) + while (resolvers.length) resolvers.shift()() + + await Promise.all(promises) + expect(peakConcurrent).toBe(2) + }) + + it("allows unbounded parallelism when semaphore is null", async () => { + scanner.semaphore = null + scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } + + let activeConcurrent = 0 + let peakConcurrent = 0 + const resolvers = [] + + scanner.scan = jest.fn(() => { + activeConcurrent++ + peakConcurrent = Math.max(peakConcurrent, activeConcurrent) + return new Promise((resolve) => { + resolvers.push(() => { + activeConcurrent-- + resolve({ isMalware: false, hash: "h" }) + }) + }) + }) + jest.spyOn(scanner, "updateStatus").mockResolvedValue() + + const promises = [] + for (let i = 0; i < 5; i++) { + promises.push( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ) + } + + await new Promise((r) => setTimeout(r, 50)) + expect(activeConcurrent).toBe(5) + expect(peakConcurrent).toBe(5) + + while (resolvers.length) resolvers.shift()() + await Promise.all(promises) + }) + + it("holds semaphore slot during scan (slot not released until complete)", async () => { + scanner.semaphore = { _max: 1, _current: 0, _queue: [], + acquire() { + if (this._current < this._max) { this._current++; return Promise.resolve() } + return new Promise((resolve) => this._queue.push(resolve)) + }, + release() { + if (this._queue.length > 0) { this._queue.shift()() } else { this._current-- } + }, + } + scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1, maxDelay: 5 } + + let scan2Started = false + const resolvers = [] + + scanner.scan = jest.fn(() => { + return new Promise((resolve) => { + resolvers.push(() => resolve({ isMalware: false, hash: "h" })) + }) + }) + jest.spyOn(scanner, "updateStatus").mockResolvedValue() + + const p1 = scanner.scanAttachmentsFile({ data: { target, keys } }) + const p2 = scanner.scanAttachmentsFile({ data: { target, keys } }).then(() => { + scan2Started = true + }) + + // Give p1 time to acquire slot + await new Promise((r) => setTimeout(r, 30)) + // p1 is running, p2 should be queued + expect(resolvers).toHaveLength(1) + + // Complete p1 + resolvers.shift()() + await new Promise((r) => setTimeout(r, 30)) + + // Now p2 should have started and its scan should be in resolvers + expect(resolvers).toHaveLength(1) + resolvers.shift()() + + await Promise.all([p1, p2]) + expect(scan2Started).toBe(true) + }) +}) From 959381cb9fed1a6be66f0d032cb72cd4d7a9cbd4 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Mon, 16 Mar 2026 16:30:31 +0100 Subject: [PATCH 02/26] refactor: enhance readability --- srv/malwareScanner.js | 192 ++++++++++++------------ srv/semaphore.js | 26 ++++ tests/unit/malwareScanner.test.js | 237 ++++++++++-------------------- 3 files changed, 193 insertions(+), 262 deletions(-) create mode 100644 srv/semaphore.js diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index a44663e7..ab72240b 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -3,33 +3,9 @@ const crypto = require("crypto") const https = require("https") const axios = require("axios") const { URL } = require("url") +const Semaphore = require("./semaphore") const LOG = cds.log("attachments") -class Semaphore { - constructor(max) { - this._max = max - this._current = 0 - this._queue = [] - } - - acquire() { - if (this._current < this._max) { - this._current++ - return Promise.resolve() - } - return new Promise((resolve) => this._queue.push(resolve)) - } - - release() { - if (this._queue.length > 0) { - const next = this._queue.shift() - next() - } else { - this._current-- - } - } -} - class MalwareScanner extends cds.ApplicationService { get credentials() { return getCredentials() @@ -182,53 +158,7 @@ class MalwareScanner extends cds.ApplicationService { try { const url = new URL(`https://${this.credentials.uri}/scan`) - const headers = {} - let httpsAgent - - if (this.credentials?.certificate && this.credentials?.key) { - LOG.debug("Using mTLS authentication for malware scanning") - - const cert = new crypto.X509Certificate(this.credentials.certificate) - const expiryDate = new Date(cert.validTo) - const now = Date.now() - - // Show warning if certificate is expired or expiring within 30 days - const msIn30Days = 30 * 24 * 60 * 60 * 1000 - - if (expiryDate.getTime() < now) { - LOG.error("Malware scanner certificate expired", { - validTo: cert.validTo, - }) - throw new Error("Malware scanner certificate expired") - } else if (expiryDate.getTime() - now < msIn30Days) { - LOG.warn("Malware scanner certificate expiring soon", { - validTo: cert.validTo, - }) - } - - httpsAgent = new https.Agent({ - cert: this.credentials.certificate, - key: this.credentials.key, - rejectUnauthorized: false, - }) - - LOG.debug("Using mTLS authorization") - } else if (this.credentials?.username && this.credentials?.password) { - LOG.warn( - "Deprecated: Basic Authentication for malware scanning is deprecated and will be removed in future releases.", - ) - headers.Authorization = - "Basic " + - Buffer.from( - `${this.credentials.username}:${this.credentials.password}`, - "binary", - ).toString("base64") - LOG.debug("Using basic authorization") - } else { - throw new Error( - "Could not find any credentials to authenticate against malware scanning service, please make sure binding and service key exists.", - ) - } + const { headers, httpsAgent } = this._buildRequestConfig() // Buffer the stream so retries can re-send the same data const chunks = [] @@ -251,7 +181,10 @@ class MalwareScanner extends cds.ApplicationService { /** @type {MalwareScanResponse} */ const responseJson = response.data const scanDuration = Date.now() - scanStartTime - LOG.debug(`Malware scan response`, { scanDuration, response: responseJson }) + LOG.debug(`Malware scan response`, { + scanDuration, + response: responseJson, + }) return { isMalware: responseJson.malwareDetected, @@ -276,8 +209,57 @@ class MalwareScanner extends cds.ApplicationService { } } + _buildRequestConfig() { + const headers = {} + let httpsAgent + + if (this.credentials?.certificate && this.credentials?.key) { + LOG.debug("Using mTLS authentication for malware scanning") + + const cert = new crypto.X509Certificate(this.credentials.certificate) + const expiryDate = new Date(cert.validTo) + const now = Date.now() + const msIn30Days = 30 * 24 * 60 * 60 * 1000 + + if (expiryDate.getTime() < now) { + LOG.error("Malware scanner certificate expired", { + validTo: cert.validTo, + }) + throw new Error("Malware scanner certificate expired") + } else if (expiryDate.getTime() - now < msIn30Days) { + LOG.warn("Malware scanner certificate expiring soon", { + validTo: cert.validTo, + }) + } + + httpsAgent = new https.Agent({ + cert: this.credentials.certificate, + key: this.credentials.key, + rejectUnauthorized: false, + }) + LOG.debug("Using mTLS authorization") + } else if (this.credentials?.username && this.credentials?.password) { + LOG.warn( + "Deprecated: Basic Authentication for malware scanning is deprecated and will be removed in future releases.", + ) + headers.Authorization = + "Basic " + + Buffer.from( + `${this.credentials.username}:${this.credentials.password}`, + "binary", + ).toString("base64") + LOG.debug("Using basic authorization") + } else { + throw new Error( + "Could not find any credentials to authenticate against malware scanning service, please make sure binding and service key exists.", + ) + } + + return { headers, httpsAgent } + } + async _scanWithRetry(url, buffer, headers, httpsAgent) { - const { enabled, maxAttempts, initialDelay, maxDelay } = this.retryConfig + const { enabled, maxAttempts } = this.retryConfig const attempts = enabled ? maxAttempts : 1 for (let attempt = 1; attempt <= attempts; attempt++) { @@ -285,7 +267,6 @@ class MalwareScanner extends cds.ApplicationService { const response = await axios.post(url.toString(), buffer, { headers, httpsAgent, - // Axios throws on non-2xx by default; we need to handle 429 ourselves validateStatus: (status) => status >= 200 && status < 300, }) if (attempt > 1) { @@ -294,39 +275,48 @@ class MalwareScanner extends cds.ApplicationService { return response } catch (err) { const status = err.response?.status - if (status !== 429 || attempt === attempts) { - if (status === 429) { - throw new Error( - `Malware scan failed after ${attempts} attempts (last status: 429)`, - ) - } - // Non-429 error or non-Axios error — rethrow as-is - const data = err.response?.data - if (data) { - const errorMsg = - JSON.stringify(data) || "Unknown error from malware scanner" - throw new Error(`Scanning failed: ${errorMsg}`) - } - throw err + const isRateLimited = status === 429 + const hasRetriesLeft = attempt < attempts + + // Only retry on 429 — all other errors fail immediately + if (isRateLimited && hasRetriesLeft) { + const delay = this._calculateRetryDelay(err, attempt) + LOG.debug( + `Malware scan received 429, retrying in ${Math.round(delay)}ms (attempt ${attempt}/${attempts})`, + ) + await new Promise((resolve) => setTimeout(resolve, delay)) + continue } - // Calculate delay for retry - let delay - const retryAfter = err.response?.headers?.["retry-after"] - if (retryAfter) { - delay = Math.min(parseInt(retryAfter, 10) * 1000, maxDelay) - } else { - const jitter = Math.random() * 500 - delay = Math.min(initialDelay * Math.pow(2, attempt - 1) + jitter, maxDelay) + if (isRateLimited) { + throw new Error( + `Malware scan failed after ${attempts} attempts (last status: 429)`, + ) } - LOG.debug( - `Malware scan received 429, retrying in ${Math.round(delay)}ms (attempt ${attempt}/${attempts})`, - ) - await new Promise((resolve) => setTimeout(resolve, delay)) + const data = err.response?.data + if (data) { + throw new Error( + `Scanning failed: ${JSON.stringify(data) || "Unknown error from malware scanner"}`, + ) + } + + throw err } } } + + _calculateRetryDelay(err, attempt) { + const { initialDelay, maxDelay } = this.retryConfig + const retryAfter = err.response?.headers?.["retry-after"] + + if (retryAfter) { + return Math.min(parseInt(retryAfter, 10) * 1000, maxDelay) + } + + const jitter = Math.random() * 500 + return Math.min(initialDelay * Math.pow(2, attempt - 1) + jitter, maxDelay) + } } module.exports = MalwareScanner diff --git a/srv/semaphore.js b/srv/semaphore.js new file mode 100644 index 00000000..fa9e2706 --- /dev/null +++ b/srv/semaphore.js @@ -0,0 +1,26 @@ +class Semaphore { + constructor(max) { + this._max = max + this._current = 0 + this._queue = [] + } + + acquire() { + if (this._current < this._max) { + this._current++ + return Promise.resolve() + } + return new Promise((resolve) => this._queue.push(resolve)) + } + + release() { + if (this._queue.length > 0) { + const next = this._queue.shift() + next() + } else { + this._current-- + } + } +} + +module.exports = Semaphore diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index 7642eed2..f3c47937 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -7,6 +7,7 @@ const axios = require("axios") jest.mock("axios") const MalwareScanner = require("../../srv/malwareScanner") +const Semaphore = require("../../srv/semaphore") let scanner let attachmentsSvc @@ -234,8 +235,24 @@ describe("getCredentials", () => { // --------------------------------------------------------------------------- describe("_scanWithRetry (retry on 429)", () => { + const scanUrl = new URL("https://host/scan") + const scanBuffer = Buffer.from("data") + const callRetry = () => + scanner._scanWithRetry(scanUrl, scanBuffer, {}, undefined) + + function make429(headers = {}) { + const err = new Error("429") + err.response = { status: 429, headers } + return err + } + + function make500(data = { error: "Internal Server Error" }) { + const err = new Error("500") + err.response = { status: 500, data } + return err + } + beforeEach(() => { - // Fast retries for tests scanner.retryConfig = { enabled: true, maxAttempts: 3, @@ -248,126 +265,68 @@ describe("_scanWithRetry (retry on 429)", () => { const successResponse = { status: 200, data: { malwareDetected: false } } axios.post.mockResolvedValueOnce(successResponse) - const result = await scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ) + const result = await callRetry() expect(result).toBe(successResponse) expect(axios.post).toHaveBeenCalledTimes(1) }) it("retries on 429 then succeeds", async () => { - const err429 = new Error("429") - err429.response = { status: 429, headers: {} } const successResponse = { status: 200, data: { malwareDetected: false } } - axios.post - .mockRejectedValueOnce(err429) + .mockRejectedValueOnce(make429()) .mockResolvedValueOnce(successResponse) - const result = await scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ) + const result = await callRetry() expect(result).toBe(successResponse) expect(axios.post).toHaveBeenCalledTimes(2) }) it("throws after exhausting all retry attempts on 429", async () => { - const err429 = new Error("429") - err429.response = { status: 429, headers: {} } - axios.post - .mockRejectedValueOnce(err429) - .mockRejectedValueOnce(err429) - .mockRejectedValueOnce(err429) + .mockRejectedValueOnce(make429()) + .mockRejectedValueOnce(make429()) + .mockRejectedValueOnce(make429()) - await expect( - scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ), - ).rejects.toThrow("Malware scan failed after 3 attempts (last status: 429)") + await expect(callRetry()).rejects.toThrow( + "Malware scan failed after 3 attempts (last status: 429)", + ) expect(axios.post).toHaveBeenCalledTimes(3) }) it("respects Retry-After header", async () => { - const err429 = new Error("429") - err429.response = { status: 429, headers: { "retry-after": "1" } } const successResponse = { status: 200, data: { malwareDetected: false } } - axios.post - .mockRejectedValueOnce(err429) + .mockRejectedValueOnce(make429({ "retry-after": "1" })) .mockResolvedValueOnce(successResponse) const start = Date.now() - await scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ) + await callRetry() // Retry-After: 1 second = 1000ms, but capped at maxDelay (10ms in test config) - // So the delay should be ~10ms, not 1000ms const elapsed = Date.now() - start expect(elapsed).toBeLessThan(500) }) it("does not retry when retry is disabled", async () => { scanner.retryConfig.enabled = false + axios.post.mockRejectedValueOnce(make429()) - const err429 = new Error("429") - err429.response = { status: 429, headers: {} } - - axios.post.mockRejectedValueOnce(err429) - - await expect( - scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ), - ).rejects.toThrow("Malware scan failed after 1 attempts (last status: 429)") + await expect(callRetry()).rejects.toThrow( + "Malware scan failed after 1 attempts (last status: 429)", + ) expect(axios.post).toHaveBeenCalledTimes(1) }) it("does not retry on non-429 errors", async () => { - const err500 = new Error("500") - err500.response = { status: 500, data: { error: "Internal Server Error" } } + axios.post.mockRejectedValueOnce(make500()) - axios.post.mockRejectedValueOnce(err500) - - await expect( - scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ), - ).rejects.toThrow("Scanning failed:") + await expect(callRetry()).rejects.toThrow("Scanning failed:") expect(axios.post).toHaveBeenCalledTimes(1) }) it("rethrows network errors without retry", async () => { - const networkErr = new Error("ECONNREFUSED") + axios.post.mockRejectedValueOnce(new Error("ECONNREFUSED")) - axios.post.mockRejectedValueOnce(networkErr) - - await expect( - scanner._scanWithRetry( - new URL("https://host/scan"), - Buffer.from("data"), - {}, - undefined, - ), - ).rejects.toThrow("ECONNREFUSED") + await expect(callRetry()).rejects.toThrow("ECONNREFUSED") expect(axios.post).toHaveBeenCalledTimes(1) }) }) @@ -379,24 +338,13 @@ describe("_scanWithRetry (retry on 429)", () => { describe("concurrency limiting", () => { const target = "ProcessorService.Incidents.attachments" const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } + const noRetry = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } beforeEach(() => { - // Ensure attachmentsSrv.get returns a truthy value so scan flow proceeds to this.scan() attachmentsSvc.get.mockResolvedValue({}) }) - it("limits concurrent scans to maxConcurrentScans", async () => { - scanner.semaphore = { _max: 2, _current: 0, _queue: [], - acquire() { - if (this._current < this._max) { this._current++; return Promise.resolve() } - return new Promise((resolve) => this._queue.push(resolve)) - }, - release() { - if (this._queue.length > 0) { this._queue.shift()() } else { this._current-- } - }, - } - scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } - + function mockScanWithConcurrencyTracking() { let activeConcurrent = 0 let peakConcurrent = 0 const resolvers = [] @@ -413,109 +361,76 @@ describe("concurrency limiting", () => { }) jest.spyOn(scanner, "updateStatus").mockResolvedValue() + return { + get activeConcurrent() { return activeConcurrent }, + get peakConcurrent() { return peakConcurrent }, + resolvers, + } + } + + it("limits concurrent scans to maxConcurrentScans", async () => { + scanner.semaphore = new Semaphore(2) + scanner.retryConfig = noRetry + const tracking = mockScanWithConcurrencyTracking() + const promises = [] for (let i = 0; i < 5; i++) { - promises.push( - scanner.scanAttachmentsFile({ data: { target, keys } }), - ) + promises.push(scanner.scanAttachmentsFile({ data: { target, keys } })) } - // Wait for first batch to start await new Promise((r) => setTimeout(r, 50)) - expect(activeConcurrent).toBe(2) - expect(resolvers).toHaveLength(2) + expect(tracking.activeConcurrent).toBe(2) + expect(tracking.resolvers).toHaveLength(2) - // Complete first two scans - resolvers.shift()() - resolvers.shift()() + tracking.resolvers.shift()() + tracking.resolvers.shift()() await new Promise((r) => setTimeout(r, 50)) + expect(tracking.activeConcurrent).toBe(2) - // Next batch should have started - expect(activeConcurrent).toBe(2) - - // Complete remaining - while (resolvers.length) resolvers.shift()() + while (tracking.resolvers.length) tracking.resolvers.shift()() await new Promise((r) => setTimeout(r, 50)) - while (resolvers.length) resolvers.shift()() + while (tracking.resolvers.length) tracking.resolvers.shift()() await Promise.all(promises) - expect(peakConcurrent).toBe(2) + expect(tracking.peakConcurrent).toBe(2) }) it("allows unbounded parallelism when semaphore is null", async () => { scanner.semaphore = null - scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } - - let activeConcurrent = 0 - let peakConcurrent = 0 - const resolvers = [] - - scanner.scan = jest.fn(() => { - activeConcurrent++ - peakConcurrent = Math.max(peakConcurrent, activeConcurrent) - return new Promise((resolve) => { - resolvers.push(() => { - activeConcurrent-- - resolve({ isMalware: false, hash: "h" }) - }) - }) - }) - jest.spyOn(scanner, "updateStatus").mockResolvedValue() + scanner.retryConfig = noRetry + const tracking = mockScanWithConcurrencyTracking() const promises = [] for (let i = 0; i < 5; i++) { - promises.push( - scanner.scanAttachmentsFile({ data: { target, keys } }), - ) + promises.push(scanner.scanAttachmentsFile({ data: { target, keys } })) } await new Promise((r) => setTimeout(r, 50)) - expect(activeConcurrent).toBe(5) - expect(peakConcurrent).toBe(5) + expect(tracking.activeConcurrent).toBe(5) + expect(tracking.peakConcurrent).toBe(5) - while (resolvers.length) resolvers.shift()() + while (tracking.resolvers.length) tracking.resolvers.shift()() await Promise.all(promises) }) it("holds semaphore slot during scan (slot not released until complete)", async () => { - scanner.semaphore = { _max: 1, _current: 0, _queue: [], - acquire() { - if (this._current < this._max) { this._current++; return Promise.resolve() } - return new Promise((resolve) => this._queue.push(resolve)) - }, - release() { - if (this._queue.length > 0) { this._queue.shift()() } else { this._current-- } - }, - } - scanner.retryConfig = { enabled: false, maxAttempts: 1, initialDelay: 1, maxDelay: 5 } + scanner.semaphore = new Semaphore(1) + scanner.retryConfig = noRetry + const tracking = mockScanWithConcurrencyTracking() let scan2Started = false - const resolvers = [] - - scanner.scan = jest.fn(() => { - return new Promise((resolve) => { - resolvers.push(() => resolve({ isMalware: false, hash: "h" })) - }) - }) - jest.spyOn(scanner, "updateStatus").mockResolvedValue() - const p1 = scanner.scanAttachmentsFile({ data: { target, keys } }) - const p2 = scanner.scanAttachmentsFile({ data: { target, keys } }).then(() => { - scan2Started = true - }) + const p2 = scanner + .scanAttachmentsFile({ data: { target, keys } }) + .then(() => { scan2Started = true }) - // Give p1 time to acquire slot await new Promise((r) => setTimeout(r, 30)) - // p1 is running, p2 should be queued - expect(resolvers).toHaveLength(1) + expect(tracking.resolvers).toHaveLength(1) - // Complete p1 - resolvers.shift()() + tracking.resolvers.shift()() await new Promise((r) => setTimeout(r, 30)) - - // Now p2 should have started and its scan should be in resolvers - expect(resolvers).toHaveLength(1) - resolvers.shift()() + expect(tracking.resolvers).toHaveLength(1) + tracking.resolvers.shift()() await Promise.all([p1, p2]) expect(scan2Started).toBe(true) From 73b1157c72df1a784b29b68c596b09244feb7f30 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Mon, 16 Mar 2026 16:33:14 +0100 Subject: [PATCH 03/26] fix: lint --- srv/malwareScanner.js | 7 ++++++- tests/unit/malwareScanner.test.js | 19 +++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index ab72240b..7a697bef 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -167,7 +167,12 @@ class MalwareScanner extends cds.ApplicationService { } const buffer = Buffer.concat(chunks) - const response = await this._scanWithRetry(url, buffer, headers, httpsAgent) + const response = await this._scanWithRetry( + url, + buffer, + headers, + httpsAgent, + ) /** * @typedef {Object} MalwareScanResponse diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index f3c47937..81270f0e 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -338,7 +338,12 @@ describe("_scanWithRetry (retry on 429)", () => { describe("concurrency limiting", () => { const target = "ProcessorService.Incidents.attachments" const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } - const noRetry = { enabled: false, maxAttempts: 1, initialDelay: 1000, maxDelay: 30000 } + const noRetry = { + enabled: false, + maxAttempts: 1, + initialDelay: 1000, + maxDelay: 30000, + } beforeEach(() => { attachmentsSvc.get.mockResolvedValue({}) @@ -362,8 +367,12 @@ describe("concurrency limiting", () => { jest.spyOn(scanner, "updateStatus").mockResolvedValue() return { - get activeConcurrent() { return activeConcurrent }, - get peakConcurrent() { return peakConcurrent }, + get activeConcurrent() { + return activeConcurrent + }, + get peakConcurrent() { + return peakConcurrent + }, resolvers, } } @@ -422,7 +431,9 @@ describe("concurrency limiting", () => { const p1 = scanner.scanAttachmentsFile({ data: { target, keys } }) const p2 = scanner .scanAttachmentsFile({ data: { target, keys } }) - .then(() => { scan2Started = true }) + .then(() => { + scan2Started = true + }) await new Promise((r) => setTimeout(r, 30)) expect(tracking.resolvers).toHaveLength(1) From f199b57a9706cfa84b18641d83c66ece9bddd6d8 Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Mon, 16 Mar 2026 17:35:01 +0100 Subject: [PATCH 04/26] docs: config for malware scan --- README.md | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/README.md b/README.md index 6d1732fc..6e492204 100755 --- a/README.md +++ b/README.md @@ -16,6 +16,8 @@ The `@cap-js/attachments` package is a [CDS plugin](https://cap.cloud.sap/docs/n - [Changes in the CDS Models](#changes-in-the-cds-models) - [Storage Targets](#storage-targets) - [Malware Scanner](#malware-scanner) + - [Rate Limit Handling (Auto-Retry)](#rate-limit-handling-auto-retry) + - [Scan Concurrency Limiting](#scan-concurrency-limiting) - [Automatic file rescanning](#automatic-file-rescanning) - [Visibility Control for Attachments UI Facet Generation](#visibility-control-for-attachments-ui-facet-generation) - [Example Usage](#example-usage) @@ -224,6 +226,62 @@ Scan status codes: > [!Note] > If the malware scanner reports a file size larger than the limit specified via [@Validation.Maximum](#specify-the-maximum-file-size) it removes the file and sets the status of the attachment metadata to failed. +#### Rate Limit Handling (Auto-Retry) + +The SAP Malware Scanning Service enforces a rate limit of 30 concurrent requests per subaccount. When this limit is exceeded, the service responds with HTTP `429 Too Many Requests`. By default, the plugin automatically retries scan requests that receive a 429 response using exponential backoff with jitter. + +You can configure the retry behavior in `package.json` or `.cdsrc.json`: + +```json +{ + "cds": { + "requires": { + "malwareScanner": { + "retry": { + "enabled": true, + "maxAttempts": 5, + "initialDelay": 1000, + "maxDelay": 30000 + } + } + } + } +} +``` + +| Option | Default | Description | +| -------------------- | ------- | ------------------------------------------------------ | +| `retry.enabled` | `true` | Enable or disable automatic retry on 429 responses | +| `retry.maxAttempts` | `5` | Total number of attempts including the initial request | +| `retry.initialDelay` | `1000` | Base delay in milliseconds before the first retry | +| `retry.maxDelay` | `30000` | Maximum delay in milliseconds between retries | + +When a 429 response includes a `Retry-After` header, the plugin respects that value (capped at `maxDelay`). Only 429 responses trigger retries — other errors fail immediately. + +To disable retry and restore the previous behavior (immediate failure on 429), set `retry.enabled` to `false`. + +#### Scan Concurrency Limiting + +To reduce pressure on the shared rate limit, the plugin limits how many scan requests run concurrently within a single process. Excess scans are queued and processed as slots become available. + +```json +{ + "cds": { + "requires": { + "malwareScanner": { + "maxConcurrentScans": 5 + } + } + } +} +``` + +| Option | Default | Description | +| -------------------- | ------- | ------------------------------------------------------------------------------------------------------ | +| `maxConcurrentScans` | `5` | Maximum number of concurrent scan requests per process. Set to `0` to disable (unbounded parallelism). | + +A scan that is retrying due to a 429 response holds its concurrency slot during the backoff wait, preventing retry storms from competing with new scans. + #### Automatic file rescanning According to the recommendation of the [Malware Scanning Service](http://help.sap.com/docs/malware-scanning-service/sap-malware-scanning-service/developing-applications-with-sap-malware-scanning-service), attachments should be rescanned automatically if the last scan is older than 3 days. This behavior can be configured in the attachments settings by specifying the `scanExpiryMs` property: From 687bf50bd6cb0239d5291fd3855f094731d28b2b Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Fri, 20 Mar 2026 08:57:04 +0100 Subject: [PATCH 05/26] refactor: fallback to official malware scan limits & readability --- srv/malwareScanner.js | 144 +++++++++++++----------------- tests/unit/malwareScanner.test.js | 124 +++++++++++++------------ 2 files changed, 130 insertions(+), 138 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index 7a697bef..41c5683e 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -22,7 +22,7 @@ class MalwareScanner extends cds.ApplicationService { initialDelay: config?.retry?.initialDelay ?? 1000, maxDelay: config?.retry?.maxDelay ?? 30000, } - const maxConcurrent = config?.maxConcurrentScans ?? 5 + const maxConcurrent = config?.maxConcurrentScans ?? 30 this.semaphore = maxConcurrent > 0 ? new Semaphore(maxConcurrent) : null return super.init() @@ -72,35 +72,19 @@ class MalwareScanner extends cds.ApplicationService { await this.updateStatus(_target, keys, "Scanning") - LOG.debug( - `Fetching file content for scanning for ${target}, ${JSON.stringify(keys)}`, - ) - const contentStream = await AttachmentsSrv.get( - model.definitions[target], - keys, - ) - - if (!contentStream) { - LOG.warn( - `Cannot fetch file content for malware scanning for ${target}, ${JSON.stringify(keys)}! Check if the file exists.`, - ) - await this.updateStatus(_target, keys, "Failed") - return - } - let res try { - res = await this.scan(contentStream) + res = await this._scanWithRetry(AttachmentsSrv, model, target, keys) } catch (err) { LOG.error( `Request to malware scanner failed for ${target}, ${JSON.stringify(keys)}`, err, ) - await this.updateStatus(target, keys, "Failed") + await this.updateStatus(_target, keys, "Failed") throw err } - let status = res.isMalware ? "Infected" : "Clean" + const status = res.isMalware ? "Infected" : "Clean" const hash = res.hash if (status === "Infected") { @@ -113,13 +97,63 @@ class MalwareScanner extends cds.ApplicationService { hash, }) } else { - LOG.debug(`Malware scan completed for ${target}, ${keys} - file is clean`) + LOG.debug( + `Malware scan completed for ${target}, ${keys} - file is clean`, + ) } - // Assign hash as another condition to ensure the correct file is marked as fine await this.updateStatus(_target, Object.assign({ hash }, keys), status) } + async _scanWithRetry(AttachmentsSrv, model, target, keys) { + const { enabled, maxAttempts } = this.retryConfig + const attempts = enabled ? maxAttempts : 1 + + for (let attempt = 1; attempt <= attempts; attempt++) { + LOG.debug( + `Fetching file content for scanning for ${target}, ${JSON.stringify(keys)}`, + ) + const contentStream = await AttachmentsSrv.get( + model.definitions[target], + keys, + ) + + if (!contentStream) { + throw new Error( + `Cannot fetch file content for malware scanning for ${target}, ${JSON.stringify(keys)}`, + ) + } + + try { + const res = await this.scan(contentStream) + if (attempt > 1) { + LOG.info(`Malware scan succeeded after ${attempt} attempts`) + } + return res + } catch (err) { + const is429 = err.response?.status === 429 + const hasRetriesLeft = attempt < attempts + + if (is429 && hasRetriesLeft) { + const delay = this._calculateRetryDelay(err, attempt) + LOG.debug( + `Malware scan received 429, retrying in ${Math.round(delay)}ms (attempt ${attempt}/${attempts})`, + ) + await new Promise((resolve) => setTimeout(resolve, delay)) + continue + } + + if (is429) { + throw new Error( + `Malware scan failed after ${attempts} attempts (last status: 429)`, + ) + } + + throw err + } + } + } + async getFileInformation(_target, keys) { const dbResult = await SELECT.one .from(_target.drafts || _target) @@ -160,19 +194,13 @@ class MalwareScanner extends cds.ApplicationService { const url = new URL(`https://${this.credentials.uri}/scan`) const { headers, httpsAgent } = this._buildRequestConfig() - // Buffer the stream so retries can re-send the same data - const chunks = [] - for await (const chunk of file) { - chunks.push(chunk) - } - const buffer = Buffer.concat(chunks) - - const response = await this._scanWithRetry( - url, - buffer, + const response = await axios.post(url.toString(), file, { headers, httpsAgent, - ) + maxBodyLength: Infinity, + maxContentLength: Infinity, + validateStatus: (status) => status >= 200 && status < 300, + }) /** * @typedef {Object} MalwareScanResponse @@ -208,7 +236,7 @@ class MalwareScanner extends cds.ApplicationService { { scanDuration, scannerUri: this.credentials?.uri }, ) file?.destroy() - return req.reject(500, "Scanning failed") + throw error } finally { file?.destroy() } @@ -263,54 +291,6 @@ class MalwareScanner extends cds.ApplicationService { return { headers, httpsAgent } } - async _scanWithRetry(url, buffer, headers, httpsAgent) { - const { enabled, maxAttempts } = this.retryConfig - const attempts = enabled ? maxAttempts : 1 - - for (let attempt = 1; attempt <= attempts; attempt++) { - try { - const response = await axios.post(url.toString(), buffer, { - headers, - httpsAgent, - validateStatus: (status) => status >= 200 && status < 300, - }) - if (attempt > 1) { - LOG.info(`Malware scan succeeded after ${attempt} attempts`) - } - return response - } catch (err) { - const status = err.response?.status - const isRateLimited = status === 429 - const hasRetriesLeft = attempt < attempts - - // Only retry on 429 — all other errors fail immediately - if (isRateLimited && hasRetriesLeft) { - const delay = this._calculateRetryDelay(err, attempt) - LOG.debug( - `Malware scan received 429, retrying in ${Math.round(delay)}ms (attempt ${attempt}/${attempts})`, - ) - await new Promise((resolve) => setTimeout(resolve, delay)) - continue - } - - if (isRateLimited) { - throw new Error( - `Malware scan failed after ${attempts} attempts (last status: 429)`, - ) - } - - const data = err.response?.data - if (data) { - throw new Error( - `Scanning failed: ${JSON.stringify(data) || "Unknown error from malware scanner"}`, - ) - } - - throw err - } - } - } - _calculateRetryDelay(err, attempt) { const { initialDelay, maxDelay } = this.retryConfig const retryAfter = err.response?.headers?.["retry-after"] diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index 81270f0e..129ad9d0 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -31,6 +31,12 @@ beforeEach(() => { cds.connect.to = jest.fn().mockResolvedValue(attachmentsSvc) scanner = new MalwareScanner() + scanner.retryConfig = { + enabled: true, + maxAttempts: 5, + initialDelay: 1000, + maxDelay: 30000, + } scanner.scan = jest.fn() }) @@ -59,7 +65,9 @@ describe("scanAttachmentsFile", () => { it("sets status Failed when content stream is null", async () => { attachmentsSvc.get.mockResolvedValue(null) jest.spyOn(scanner, "updateStatus").mockResolvedValue() - await scanner.scanAttachmentsFile({ data: { target, keys } }) + await expect( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ).rejects.toThrow("Cannot fetch file content") expect(scanner.updateStatus).toHaveBeenLastCalledWith( expect.anything(), keys, @@ -231,14 +239,12 @@ describe("getCredentials", () => { }) // --------------------------------------------------------------------------- -// _scanWithRetry — retry on 429 +// 429 retry at scanAttachmentsFile level (stream re-fetch) // --------------------------------------------------------------------------- -describe("_scanWithRetry (retry on 429)", () => { - const scanUrl = new URL("https://host/scan") - const scanBuffer = Buffer.from("data") - const callRetry = () => - scanner._scanWithRetry(scanUrl, scanBuffer, {}, undefined) +describe("429 retry (stream re-fetch)", () => { + const target = "ProcessorService.Incidents.attachments" + const keys = { up__ID: cds.utils.uuid(), ID: cds.utils.uuid() } function make429(headers = {}) { const err = new Error("429") @@ -246,12 +252,6 @@ describe("_scanWithRetry (retry on 429)", () => { return err } - function make500(data = { error: "Internal Server Error" }) { - const err = new Error("500") - err.response = { status: 500, data } - return err - } - beforeEach(() => { scanner.retryConfig = { enabled: true, @@ -259,75 +259,87 @@ describe("_scanWithRetry (retry on 429)", () => { initialDelay: 1, maxDelay: 10, } + jest.spyOn(scanner, "updateStatus").mockResolvedValue() }) - it("returns response on first success (no retry needed)", async () => { - const successResponse = { status: 200, data: { malwareDetected: false } } - axios.post.mockResolvedValueOnce(successResponse) + it("fetches stream once on first success (no retry needed)", async () => { + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan.mockResolvedValue({ isMalware: false, hash: "h" }) - const result = await callRetry() - expect(result).toBe(successResponse) - expect(axios.post).toHaveBeenCalledTimes(1) + await scanner.scanAttachmentsFile({ data: { target, keys } }) + expect(attachmentsSvc.get).toHaveBeenCalledTimes(1) + expect(scanner.scan).toHaveBeenCalledTimes(1) }) - it("retries on 429 then succeeds", async () => { - const successResponse = { status: 200, data: { malwareDetected: false } } - axios.post + it("re-fetches stream on 429 then succeeds", async () => { + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan .mockRejectedValueOnce(make429()) - .mockResolvedValueOnce(successResponse) + .mockResolvedValueOnce({ isMalware: false, hash: "h" }) - const result = await callRetry() - expect(result).toBe(successResponse) - expect(axios.post).toHaveBeenCalledTimes(2) + await scanner.scanAttachmentsFile({ data: { target, keys } }) + expect(attachmentsSvc.get).toHaveBeenCalledTimes(2) + expect(scanner.scan).toHaveBeenCalledTimes(2) }) - it("throws after exhausting all retry attempts on 429", async () => { - axios.post + it("sets status Failed after exhausting all retry attempts on 429", async () => { + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan .mockRejectedValueOnce(make429()) .mockRejectedValueOnce(make429()) .mockRejectedValueOnce(make429()) - await expect(callRetry()).rejects.toThrow( - "Malware scan failed after 3 attempts (last status: 429)", + await expect( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ).rejects.toThrow("Malware scan failed after 3 attempts (last status: 429)") + expect(attachmentsSvc.get).toHaveBeenCalledTimes(3) + expect(scanner.updateStatus).toHaveBeenLastCalledWith( + expect.anything(), + keys, + "Failed", + ) + }) + + it("does not retry on non-429 errors", async () => { + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan.mockRejectedValue(new Error("boom")) + + await expect( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ).rejects.toThrow("boom") + expect(attachmentsSvc.get).toHaveBeenCalledTimes(1) + expect(scanner.scan).toHaveBeenCalledTimes(1) + expect(scanner.updateStatus).toHaveBeenLastCalledWith( + expect.anything(), + keys, + "Failed", ) - expect(axios.post).toHaveBeenCalledTimes(3) }) - it("respects Retry-After header", async () => { - const successResponse = { status: 200, data: { malwareDetected: false } } - axios.post + it("respects Retry-After header (capped at maxDelay)", async () => { + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan .mockRejectedValueOnce(make429({ "retry-after": "1" })) - .mockResolvedValueOnce(successResponse) + .mockResolvedValueOnce({ isMalware: false, hash: "h" }) const start = Date.now() - await callRetry() - // Retry-After: 1 second = 1000ms, but capped at maxDelay (10ms in test config) + await scanner.scanAttachmentsFile({ data: { target, keys } }) + // Retry-After: 1s = 1000ms, capped at maxDelay (10ms in test config) const elapsed = Date.now() - start expect(elapsed).toBeLessThan(500) + expect(attachmentsSvc.get).toHaveBeenCalledTimes(2) }) it("does not retry when retry is disabled", async () => { scanner.retryConfig.enabled = false - axios.post.mockRejectedValueOnce(make429()) - - await expect(callRetry()).rejects.toThrow( - "Malware scan failed after 1 attempts (last status: 429)", - ) - expect(axios.post).toHaveBeenCalledTimes(1) - }) - - it("does not retry on non-429 errors", async () => { - axios.post.mockRejectedValueOnce(make500()) - - await expect(callRetry()).rejects.toThrow("Scanning failed:") - expect(axios.post).toHaveBeenCalledTimes(1) - }) - - it("rethrows network errors without retry", async () => { - axios.post.mockRejectedValueOnce(new Error("ECONNREFUSED")) + attachmentsSvc.get.mockResolvedValue({}) + scanner.scan.mockRejectedValue(make429()) - await expect(callRetry()).rejects.toThrow("ECONNREFUSED") - expect(axios.post).toHaveBeenCalledTimes(1) + await expect( + scanner.scanAttachmentsFile({ data: { target, keys } }), + ).rejects.toThrow("Malware scan failed after 1 attempts (last status: 429)") + expect(attachmentsSvc.get).toHaveBeenCalledTimes(1) + expect(scanner.scan).toHaveBeenCalledTimes(1) }) }) From a2d924e1e83958f4e2f6827921b33edaa426002b Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Tue, 24 Mar 2026 10:43:20 +0100 Subject: [PATCH 06/26] fix: lint --- srv/malwareScanner.js | 4 +--- tests/unit/malwareScanner.test.js | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index 41c5683e..ca1a783f 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -97,9 +97,7 @@ class MalwareScanner extends cds.ApplicationService { hash, }) } else { - LOG.debug( - `Malware scan completed for ${target}, ${keys} - file is clean`, - ) + LOG.debug(`Malware scan completed for ${target}, ${keys} - file is clean`) } await this.updateStatus(_target, Object.assign({ hash }, keys), status) diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index 129ad9d0..79f78256 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -3,7 +3,6 @@ const cds = require("@sap/cds") const { join } = cds.utils.path cds.test(join(__dirname, "../incidents-app")) -const axios = require("axios") jest.mock("axios") const MalwareScanner = require("../../srv/malwareScanner") From 69920471ed4fa5fcd663abade06286ebdbc0191b Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Tue, 24 Mar 2026 11:17:04 +0100 Subject: [PATCH 07/26] feat: only compute hash once when remote backends are used --- srv/basic.js | 15 ++++++++++----- srv/object-store.js | 3 +++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/srv/basic.js b/srv/basic.js index 4ab063dc..fcac66cf 100644 --- a/srv/basic.js +++ b/srv/basic.js @@ -82,11 +82,16 @@ class AttachmentsService extends cds.Service { res = await Promise.all( data.map(async (d) => { const res = await UPSERT(d).into(attachments) - const attachmentForHash = await this.get(attachments, { ID: d.ID }) - // If this is just the PUT for metadata, there is not yet any file to retrieve - if (attachmentForHash) { - const hash = await computeHash(attachmentForHash) - await this.update(attachments, { ID: d.ID }, { hash }) + // When scanning is enabled, skip hash computation here — the malware + // scanner returns SHA-256 in its response and writes the hash itself. + // This avoids a redundant file read (expensive for object store backends). + const scanEnabled = cds.env.requires?.attachments?.scan !== false + if (!scanEnabled || !this._skipInlineHash) { + const attachmentForHash = await this.get(attachments, { ID: d.ID }) + if (attachmentForHash) { + const hash = await computeHash(attachmentForHash) + await this.update(attachments, { ID: d.ID }, { hash }) + } } return res }), diff --git a/srv/object-store.js b/srv/object-store.js index d3165224..f7febf75 100644 --- a/srv/object-store.js +++ b/srv/object-store.js @@ -7,6 +7,9 @@ module.exports = class RemoteAttachmentsService extends require("./basic") { objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind separateObjectStore = this.isMultiTenancyEnabled && this.objectStoreKind === "separate" + // Skip inline hash computation in put() — the malware scanner already + // returns SHA-256, avoiding a redundant remote file download from object store. + _skipInlineHash = true init() { LOG.debug(`${this.constructor.name} initialization`, { From 3cb23b4dc75f976a9d3c8adf9802f918b61dea3a Mon Sep 17 00:00:00 2001 From: Ansgar Lichter Date: Tue, 24 Mar 2026 14:43:11 +0100 Subject: [PATCH 08/26] fix: add cause to thrown error --- srv/malwareScanner.js | 1 + 1 file changed, 1 insertion(+) diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index ca1a783f..ef19a04c 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -144,6 +144,7 @@ class MalwareScanner extends cds.ApplicationService { if (is429) { throw new Error( `Malware scan failed after ${attempts} attempts (last status: 429)`, + { cause: err }, ) } From 7e4d64924471af3ff3751706cd8cfde4d36ca850 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 14:47:52 +0100 Subject: [PATCH 09/26] Move config to package.json --- README.md | 8 +++----- package.json | 6 ++++++ srv/malwareScanner.js | 10 +++++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 6e492204..4fc25fcd 100755 --- a/README.md +++ b/README.md @@ -238,7 +238,6 @@ You can configure the retry behavior in `package.json` or `.cdsrc.json`: "requires": { "malwareScanner": { "retry": { - "enabled": true, "maxAttempts": 5, "initialDelay": 1000, "maxDelay": 30000 @@ -251,14 +250,13 @@ You can configure the retry behavior in `package.json` or `.cdsrc.json`: | Option | Default | Description | | -------------------- | ------- | ------------------------------------------------------ | -| `retry.enabled` | `true` | Enable or disable automatic retry on 429 responses | | `retry.maxAttempts` | `5` | Total number of attempts including the initial request | | `retry.initialDelay` | `1000` | Base delay in milliseconds before the first retry | | `retry.maxDelay` | `30000` | Maximum delay in milliseconds between retries | When a 429 response includes a `Retry-After` header, the plugin respects that value (capped at `maxDelay`). Only 429 responses trigger retries — other errors fail immediately. -To disable retry and restore the previous behavior (immediate failure on 429), set `retry.enabled` to `false`. +To disable retry and restore the previous behavior (immediate failure on 429), set `retry` to `false`. #### Scan Concurrency Limiting @@ -269,7 +267,7 @@ To reduce pressure on the shared rate limit, the plugin limits how many scan req "cds": { "requires": { "malwareScanner": { - "maxConcurrentScans": 5 + "maxConcurrentScans": 10 } } } @@ -278,7 +276,7 @@ To reduce pressure on the shared rate limit, the plugin limits how many scan req | Option | Default | Description | | -------------------- | ------- | ------------------------------------------------------------------------------------------------------ | -| `maxConcurrentScans` | `5` | Maximum number of concurrent scan requests per process. Set to `0` to disable (unbounded parallelism). | +| `maxConcurrentScans` | `30` | Maximum number of concurrent scan requests per process. Set to `0` to disable (unbounded parallelism). | A scan that is retrying due to a 429 response holds its concurrency slot during the backoff wait, preventing retry storms from competing with new scans. diff --git a/package.json b/package.json index 0848c552..ff1045c9 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,12 @@ "cds": { "requires": { "malwareScanner": { + "retry": { + "maxAttempts": 5, + "initialDelay": 1000, + "maxDelay": 30000, + "maxConcurrentScans": 30 + }, "vcap": { "label": "malware-scanner" } diff --git a/srv/malwareScanner.js b/srv/malwareScanner.js index ef19a04c..0df1709a 100644 --- a/srv/malwareScanner.js +++ b/srv/malwareScanner.js @@ -17,12 +17,12 @@ class MalwareScanner extends cds.ApplicationService { const config = cds.env.requires?.malwareScanner this.retryConfig = { - enabled: config?.retry?.enabled ?? true, - maxAttempts: config?.retry?.maxAttempts ?? 5, - initialDelay: config?.retry?.initialDelay ?? 1000, - maxDelay: config?.retry?.maxDelay ?? 30000, + enabled: !!config?.retry, + maxAttempts: config?.retry?.maxAttempts, + initialDelay: config?.retry?.initialDelay, + maxDelay: config?.retry?.maxDelay, } - const maxConcurrent = config?.maxConcurrentScans ?? 30 + const maxConcurrent = config?.maxConcurrentScans this.semaphore = maxConcurrent > 0 ? new Semaphore(maxConcurrent) : null return super.init() From 367ffd25da94080058b3bebc7b72e68807d08234 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 24 Mar 2026 14:48:41 +0100 Subject: [PATCH 10/26] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4fc25fcd..098a4cc6 100755 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ To reduce pressure on the shared rate limit, the plugin limits how many scan req | Option | Default | Description | | -------------------- | ------- | ------------------------------------------------------------------------------------------------------ | -| `maxConcurrentScans` | `30` | Maximum number of concurrent scan requests per process. Set to `0` to disable (unbounded parallelism). | +| `maxConcurrentScans` | `30` | Maximum number of concurrent scan requests per process. Set to `0` to disable (unbounded parallelism). | A scan that is retrying due to a 429 response holds its concurrency slot during the backoff wait, preventing retry storms from competing with new scans. From 220a82580bb563dbb600dff5914eb76f07052150 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:00:32 +0200 Subject: [PATCH 11/26] Split malware scanner and attachments for improved readbility --- package.json | 16 +++++------ srv/{ => attachments}/aws-s3.js | 4 +-- srv/{ => attachments}/azure-blob-storage.js | 4 +-- srv/{ => attachments}/basic.js | 2 +- srv/{ => attachments}/gcp.js | 4 +-- srv/{ => attachments}/object-store.js | 0 srv/{ => attachments}/standard.js | 2 +- .../malwareScanner-mocked.cds | 0 .../malwareScanner-mocked.js | 0 srv/{ => malware-scanner}/malwareScanner.cds | 0 srv/{ => malware-scanner}/malwareScanner.js | 27 +++++++++++++------ srv/{ => malware-scanner}/semaphore.js | 0 tests/unit/malwareScanner.test.js | 6 ++--- tests/unit/unitTests.test.js | 4 +-- 14 files changed, 39 insertions(+), 30 deletions(-) rename srv/{ => attachments}/aws-s3.js (99%) rename srv/{ => attachments}/azure-blob-storage.js (99%) rename srv/{ => attachments}/basic.js (99%) rename srv/{ => attachments}/gcp.js (99%) rename srv/{ => attachments}/object-store.js (100%) rename srv/{ => attachments}/standard.js (75%) rename srv/{ => malware-scanner}/malwareScanner-mocked.cds (100%) rename srv/{ => malware-scanner}/malwareScanner-mocked.js (100%) rename srv/{ => malware-scanner}/malwareScanner.cds (100%) rename srv/{ => malware-scanner}/malwareScanner.js (94%) rename srv/{ => malware-scanner}/semaphore.js (100%) diff --git a/package.json b/package.json index 915eec82..fb49bfb3 100644 --- a/package.json +++ b/package.json @@ -57,28 +57,28 @@ }, "kinds": { "malwareScanner-mock": { - "model": "@cap-js/attachments/srv/malwareScanner-mocked" + "model": "@cap-js/attachments/srv/malware-scanner/malwareScanner-mocked" }, "malwareScanner-mocked": { - "model": "@cap-js/attachments/srv/malwareScanner-mocked" + "model": "@cap-js/attachments/srv/malware-scanner/malwareScanner-mocked" }, "malwareScanner-btp": { - "model": "@cap-js/attachments/srv/malwareScanner" + "model": "@cap-js/attachments/srv/malware-scanner/malwareScanner" }, "attachments-db": { - "impl": "@cap-js/attachments/srv/basic" + "impl": "@cap-js/attachments/srv/attachments/basic" }, "attachments-standard": { - "impl": "@cap-js/attachments/srv/standard" + "impl": "@cap-js/attachments/srv/attachments/standard" }, "attachments-s3": { - "impl": "@cap-js/attachments/srv/aws-s3" + "impl": "@cap-js/attachments/srv/attachments/aws-s3" }, "attachments-azure": { - "impl": "@cap-js/attachments/srv/azure-blob-storage" + "impl": "@cap-js/attachments/srv/attachments/azure-blob-storage" }, "attachments-gcp": { - "impl": "@cap-js/attachments/srv/gcp" + "impl": "@cap-js/attachments/srv/attachments/gcp" } }, "serviceManager": { diff --git a/srv/aws-s3.js b/srv/attachments/aws-s3.js similarity index 99% rename from srv/aws-s3.js rename to srv/attachments/aws-s3.js index 7afe1592..dce8a434 100644 --- a/srv/aws-s3.js +++ b/srv/attachments/aws-s3.js @@ -7,12 +7,12 @@ const { const { Upload } = require("@aws-sdk/lib-storage") const cds = require("@sap/cds") const LOG = cds.log("attachments") -const utils = require("../lib/helper") +const utils = require("../../lib/helper") const { MAX_FILE_SIZE, sizeInBytes, createSizeCheckHandler, -} = require("../lib/helper") +} = require("../../lib/helper") module.exports = class AWSAttachmentsService extends require("./object-store") { /** diff --git a/srv/azure-blob-storage.js b/srv/attachments/azure-blob-storage.js similarity index 99% rename from srv/azure-blob-storage.js rename to srv/attachments/azure-blob-storage.js index 11e5128d..e962ba1f 100644 --- a/srv/azure-blob-storage.js +++ b/srv/attachments/azure-blob-storage.js @@ -2,12 +2,12 @@ const { BlobServiceClient } = require("@azure/storage-blob") const { AbortController } = require("abort-controller") const cds = require("@sap/cds") const LOG = cds.log("attachments") -const utils = require("../lib/helper") +const utils = require("../../lib/helper") const { MAX_FILE_SIZE, sizeInBytes, createSizeCheckHandler, -} = require("../lib/helper") +} = require("../../lib/helper") module.exports = class AzureAttachmentsService extends ( require("./object-store") diff --git a/srv/basic.js b/srv/attachments/basic.js similarity index 99% rename from srv/basic.js rename to srv/attachments/basic.js index fcac66cf..8b40bb28 100644 --- a/srv/basic.js +++ b/srv/attachments/basic.js @@ -4,7 +4,7 @@ const { computeHash, traverseEntity, buildBackAssocChain, -} = require("../lib/helper") +} = require("../../lib/helper") class AttachmentsService extends cds.Service { init() { diff --git a/srv/gcp.js b/srv/attachments/gcp.js similarity index 99% rename from srv/gcp.js rename to srv/attachments/gcp.js index 209a791c..4c520852 100644 --- a/srv/gcp.js +++ b/srv/attachments/gcp.js @@ -1,12 +1,12 @@ const { Storage } = require("@google-cloud/storage") const cds = require("@sap/cds") const LOG = cds.log("attachments") -const utils = require("../lib/helper") +const utils = require("../../lib/helper") const { MAX_FILE_SIZE, sizeInBytes, createSizeCheckHandler, -} = require("../lib/helper") +} = require("../../lib/helper") module.exports = class GoogleAttachmentsService extends ( require("./object-store") diff --git a/srv/object-store.js b/srv/attachments/object-store.js similarity index 100% rename from srv/object-store.js rename to srv/attachments/object-store.js diff --git a/srv/standard.js b/srv/attachments/standard.js similarity index 75% rename from srv/standard.js rename to srv/attachments/standard.js index 6f63cbf5..135f6998 100644 --- a/srv/standard.js +++ b/srv/attachments/standard.js @@ -1,4 +1,4 @@ -const { getAttachmentKind } = require("../lib/helper") +const { getAttachmentKind } = require("../../lib/helper") const LOG = require("@sap/cds").log("attachments") LOG.info(`Using ${getAttachmentKind()} for attachments management.`) diff --git a/srv/malwareScanner-mocked.cds b/srv/malware-scanner/malwareScanner-mocked.cds similarity index 100% rename from srv/malwareScanner-mocked.cds rename to srv/malware-scanner/malwareScanner-mocked.cds diff --git a/srv/malwareScanner-mocked.js b/srv/malware-scanner/malwareScanner-mocked.js similarity index 100% rename from srv/malwareScanner-mocked.js rename to srv/malware-scanner/malwareScanner-mocked.js diff --git a/srv/malwareScanner.cds b/srv/malware-scanner/malwareScanner.cds similarity index 100% rename from srv/malwareScanner.cds rename to srv/malware-scanner/malwareScanner.cds diff --git a/srv/malwareScanner.js b/srv/malware-scanner/malwareScanner.js similarity index 94% rename from srv/malwareScanner.js rename to srv/malware-scanner/malwareScanner.js index 0df1709a..d19efcae 100644 --- a/srv/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -1,7 +1,6 @@ const cds = require("@sap/cds") const crypto = require("crypto") const https = require("https") -const axios = require("axios") const { URL } = require("url") const Semaphore = require("./semaphore") const LOG = cds.log("attachments") @@ -100,6 +99,7 @@ class MalwareScanner extends cds.ApplicationService { LOG.debug(`Malware scan completed for ${target}, ${keys} - file is clean`) } + // Assign hash as another condition to ensure the correct file is marked as fine await this.updateStatus(_target, Object.assign({ hash }, keys), status) } @@ -193,13 +193,24 @@ class MalwareScanner extends cds.ApplicationService { const url = new URL(`https://${this.credentials.uri}/scan`) const { headers, httpsAgent } = this._buildRequestConfig() - const response = await axios.post(url.toString(), file, { + const fetchOptions = { + method: "POST", headers, - httpsAgent, - maxBodyLength: Infinity, - maxContentLength: Infinity, - validateStatus: (status) => status >= 200 && status < 300, - }) + body: file, + duplex: "half", + } + if (httpsAgent) fetchOptions.agent = httpsAgent + + const response = await fetch(url.toString(), fetchOptions) + + if (!response.ok) { + const error = new Error(`Malware scan request failed with status ${response.status}`) + error.response = { + status: response.status, + headers: Object.fromEntries(response.headers), + } + throw error + } /** * @typedef {Object} MalwareScanResponse @@ -211,7 +222,7 @@ class MalwareScanner extends cds.ApplicationService { * @property {string} SHA256 - SHA-256 hash of the scanned file. */ /** @type {MalwareScanResponse} */ - const responseJson = response.data + const responseJson = await response.json() const scanDuration = Date.now() - scanStartTime LOG.debug(`Malware scan response`, { scanDuration, diff --git a/srv/semaphore.js b/srv/malware-scanner/semaphore.js similarity index 100% rename from srv/semaphore.js rename to srv/malware-scanner/semaphore.js diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index 79f78256..04828384 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -3,10 +3,8 @@ const cds = require("@sap/cds") const { join } = cds.utils.path cds.test(join(__dirname, "../incidents-app")) -jest.mock("axios") - -const MalwareScanner = require("../../srv/malwareScanner") -const Semaphore = require("../../srv/semaphore") +const MalwareScanner = require("../../srv/malware-scanner/malwareScanner") +const Semaphore = require("../../srv/malware-scanner/semaphore") let scanner let attachmentsSvc diff --git a/tests/unit/unitTests.test.js b/tests/unit/unitTests.test.js index 31a412c7..112a36e0 100644 --- a/tests/unit/unitTests.test.js +++ b/tests/unit/unitTests.test.js @@ -21,8 +21,8 @@ global.fetch = jest.fn(() => jest.mock("axios") // Mock individual functions used in malwareScanner since it imports logger -jest.doMock("../../srv/malwareScanner", () => { - const original = jest.requireActual("../../srv/malwareScanner") +jest.doMock("../../srv/malware-scanner/malwareScanner", () => { + const original = jest.requireActual("../../srv/malware-scanner/malwareScanner") return { ...original, // Override streamToString to return a simple string From d6a8a1c23bf1055d6c4ae638ec25ce190a8680d4 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:11:27 +0200 Subject: [PATCH 12/26] Formatting --- srv/malware-scanner/malwareScanner.js | 4 +++- tests/unit/unitTests.test.js | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index d19efcae..ac994d6c 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -204,7 +204,9 @@ class MalwareScanner extends cds.ApplicationService { const response = await fetch(url.toString(), fetchOptions) if (!response.ok) { - const error = new Error(`Malware scan request failed with status ${response.status}`) + const error = new Error( + `Malware scan request failed with status ${response.status}`, + ) error.response = { status: response.status, headers: Object.fromEntries(response.headers), diff --git a/tests/unit/unitTests.test.js b/tests/unit/unitTests.test.js index 112a36e0..4b86a2dc 100644 --- a/tests/unit/unitTests.test.js +++ b/tests/unit/unitTests.test.js @@ -22,7 +22,9 @@ jest.mock("axios") // Mock individual functions used in malwareScanner since it imports logger jest.doMock("../../srv/malware-scanner/malwareScanner", () => { - const original = jest.requireActual("../../srv/malware-scanner/malwareScanner") + const original = jest.requireActual( + "../../srv/malware-scanner/malwareScanner", + ) return { ...original, // Override streamToString to return a simple string From bce58c366be39f05740112af459b121ff34cca63 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:16:39 +0200 Subject: [PATCH 13/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index ac994d6c..c469de41 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -125,7 +125,7 @@ class MalwareScanner extends cds.ApplicationService { try { const res = await this.scan(contentStream) if (attempt > 1) { - LOG.info(`Malware scan succeeded after ${attempt} attempts`) + LOG.info(`Malware scan for ${target}, ${keys} succeeded after ${attempt} attempts`) } return res } catch (err) { From 273ee8a620f0920a94fe540fad22a791ee9c43e8 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:20:21 +0200 Subject: [PATCH 14/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index c469de41..de5dcef3 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -125,7 +125,9 @@ class MalwareScanner extends cds.ApplicationService { try { const res = await this.scan(contentStream) if (attempt > 1) { - LOG.info(`Malware scan for ${target}, ${keys} succeeded after ${attempt} attempts`) + LOG.info( + `Malware scan for ${target}, ${JSON.stringify(keys)} succeeded after ${attempt} attempts`, + ) } return res } catch (err) { From cc2dd91dbd4be0f479b1a25c5d8cfcf4661b6b33 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:25:57 +0200 Subject: [PATCH 15/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index de5dcef3..9afc1ce9 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -155,31 +155,31 @@ class MalwareScanner extends cds.ApplicationService { } } - 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 293631cc4d15b7ff7c9bdb6fe5e468100c32d43e Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:26:05 +0200 Subject: [PATCH 16/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index 9afc1ce9..f86c481b 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -166,9 +166,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 5b49103fb6626a0403c39f0a1a56c56fdf103185 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:31:28 +0200 Subject: [PATCH 17/26] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3918e42a..d179acc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). ## Version 3.12.0 - Upcoming +### Added +- A maximum concurrent amount of scans can now be configured for the malware scanner. + +### Changed + +- The retry logic for the malware scanner was improved to be more robust under high loads. + ### Fixed - Wrong file name being shown when rejecting an attachment due to file size. From b12b1391af14da562bea9242462543bbc5e906f7 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:31:34 +0200 Subject: [PATCH 18/26] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d179acc1..9a5c9723 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). ## Version 3.12.0 - Upcoming ### Added + - A maximum concurrent amount of scans can now be configured for the malware scanner. ### Changed From dce9470bbd109f9cbb1929aa694e5089ab383bf4 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 13:41:48 +0200 Subject: [PATCH 19/26] Update auditLogging.test.js --- tests/unit/auditLogging.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/auditLogging.test.js b/tests/unit/auditLogging.test.js index be223bd1..ab5a25b6 100644 --- a/tests/unit/auditLogging.test.js +++ b/tests/unit/auditLogging.test.js @@ -76,7 +76,7 @@ describe("Audit logging when audit-logging is disabled", () => { cds.env.requires["audit-log"] = false // Create a fresh AttachmentsService instance with audit logging disabled - const AttachmentsService = require("../../srv/basic") + const AttachmentsService = require("../../srv/attachments/basic") const svc = new AttachmentsService() svc.model = cds.model // Stub super.init() to avoid full service bootstrap From 346ec96e851b83ac859d72aa6326d04bca5b0190 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 14:35:53 +0200 Subject: [PATCH 20/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 57 ++++++++++++++++----------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index f86c481b..c4d67c13 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -191,25 +191,31 @@ class MalwareScanner extends cds.ApplicationService { try { const url = new URL(`https://${this.credentials.uri}/scan`) - const { headers, httpsAgent } = this._buildRequestConfig() - - const fetchOptions = { - method: "POST", - headers, - body: file, - duplex: "half", - } - if (httpsAgent) fetchOptions.agent = httpsAgent - - const response = await fetch(url.toString(), fetchOptions) + const requestOptions = this._buildRequestOptions(url) + + const response = await new Promise((resolve, reject) => { + const httpReq = https.request(requestOptions, (res) => { + let data = "" + res.on("data", (chunk) => (data += chunk)) + res.on("end", () => { + resolve({ + status: res.statusCode, + headers: res.headers, + data, + }) + }) + }) + httpReq.on("error", reject) + file.pipe(httpReq) + }) - if (!response.ok) { + if (response.status < 200 || response.status >= 300) { const error = new Error( `Malware scan request failed with status ${response.status}`, ) error.response = { status: response.status, - headers: Object.fromEntries(response.headers), + headers: response.headers, } throw error } @@ -224,7 +230,7 @@ class MalwareScanner extends cds.ApplicationService { * @property {string} SHA256 - SHA-256 hash of the scanned file. */ /** @type {MalwareScanResponse} */ - const responseJson = await response.json() + const responseJson = JSON.parse(response.data) const scanDuration = Date.now() - scanStartTime LOG.debug(`Malware scan response`, { scanDuration, @@ -254,9 +260,14 @@ class MalwareScanner extends cds.ApplicationService { } } - _buildRequestConfig() { - const headers = {} - let httpsAgent + _buildRequestOptions(url) { + const requestOptions = { + method: "POST", + hostname: url.hostname, + port: url.port || 443, + path: url.pathname, + headers: {}, + } if (this.credentials?.certificate && this.credentials?.key) { LOG.debug("Using mTLS authentication for malware scanning") @@ -277,17 +288,15 @@ class MalwareScanner extends cds.ApplicationService { }) } - httpsAgent = new https.Agent({ - cert: this.credentials.certificate, - key: this.credentials.key, - rejectUnauthorized: false, - }) + requestOptions.cert = this.credentials.certificate + requestOptions.key = this.credentials.key + requestOptions.rejectUnauthorized = false LOG.debug("Using mTLS authorization") } else if (this.credentials?.username && this.credentials?.password) { LOG.warn( "Deprecated: Basic Authentication for malware scanning is deprecated and will be removed in future releases.", ) - headers.Authorization = + requestOptions.headers.Authorization = "Basic " + Buffer.from( `${this.credentials.username}:${this.credentials.password}`, @@ -300,7 +309,7 @@ class MalwareScanner extends cds.ApplicationService { ) } - return { headers, httpsAgent } + return requestOptions } _calculateRetryDelay(err, attempt) { From 68a5f20fce992818df417f6c7128ab9ebd8ad77d Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 17:10:29 +0200 Subject: [PATCH 21/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index c4d67c13..bb296dce 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -194,30 +194,28 @@ class MalwareScanner extends cds.ApplicationService { const requestOptions = this._buildRequestOptions(url) const response = await new Promise((resolve, reject) => { - const httpReq = https.request(requestOptions, (res) => { + const req = https.request(requestOptions, (res) => { let data = "" res.on("data", (chunk) => (data += chunk)) res.on("end", () => { resolve({ status: res.statusCode, - headers: res.headers, + ok: res.statusCode >= 200 && res.statusCode < 300, data, }) }) }) - httpReq.on("error", reject) - file.pipe(httpReq) + req.on("error", reject) + file.pipe(req) }) - if (response.status < 200 || response.status >= 300) { - const error = new Error( - `Malware scan request failed with status ${response.status}`, - ) - error.response = { - status: response.status, - headers: response.headers, - } - throw error + if (!response.ok) { + const json = JSON.parse(response.data || "{}") + const errorMsg = + JSON.stringify(json) || + response.statusText || + "Unknown error from malware scanner" + return req.reject(response.status, `Scanning failed: ${errorMsg}`) } /** From 8b7ddb377e789ea3e7ca31afa6264b7d981881b4 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Tue, 14 Apr 2026 18:35:49 +0200 Subject: [PATCH 22/26] Update malwareScanner.js --- srv/malware-scanner/malwareScanner.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index bb296dce..c4d67c13 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -194,28 +194,30 @@ class MalwareScanner extends cds.ApplicationService { const requestOptions = this._buildRequestOptions(url) const response = await new Promise((resolve, reject) => { - const req = https.request(requestOptions, (res) => { + const httpReq = https.request(requestOptions, (res) => { let data = "" res.on("data", (chunk) => (data += chunk)) res.on("end", () => { resolve({ status: res.statusCode, - ok: res.statusCode >= 200 && res.statusCode < 300, + headers: res.headers, data, }) }) }) - req.on("error", reject) - file.pipe(req) + httpReq.on("error", reject) + file.pipe(httpReq) }) - if (!response.ok) { - const json = JSON.parse(response.data || "{}") - const errorMsg = - JSON.stringify(json) || - response.statusText || - "Unknown error from malware scanner" - return req.reject(response.status, `Scanning failed: ${errorMsg}`) + if (response.status < 200 || response.status >= 300) { + const error = new Error( + `Malware scan request failed with status ${response.status}`, + ) + error.response = { + status: response.status, + headers: response.headers, + } + throw error } /** From f8e1d762be3741227442859497dcc582a7f6792d Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 15 Apr 2026 14:53:29 +0200 Subject: [PATCH 23/26] Fix --- srv/malware-scanner/malwareScanner.js | 31 ++++++++++++++++++++------- tests/incidents-app/package.json | 7 +++--- tests/integration/attachments.test.js | 12 ++--------- tests/unit/malwareScanner.test.js | 2 ++ tests/utils/testUtils.js | 2 ++ 5 files changed, 32 insertions(+), 22 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index c4d67c13..41a61f1c 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -100,7 +100,15 @@ class MalwareScanner extends cds.ApplicationService { } // Assign hash as another condition to ensure the correct file is marked as fine - await this.updateStatus(_target, Object.assign({ hash }, keys), status) + await this.updateStatus(_target, ['(', {ref: ['hash']}, '=', {val: hash}, 'or', {ref: ['hash']}, 'is', 'null', ')', 'and', {xpr: Object.keys(keys).reduce((acc, key) => { + if (acc.length) acc.push('and') + acc.push( + {ref: [key]}, + '=', + {val: keys[key]} + ) + return acc; + }, [])}], status, hash) } async _scanWithRetry(AttachmentsSrv, model, target, keys) { @@ -163,21 +171,28 @@ class MalwareScanner extends cds.ApplicationService { return dbResult } - async updateStatus(target, keys, status) { + async updateStatus(target, where, status, hash) { + const updateObject = { status }; + if (status !== 'Scanning') { + updateObject.lastScan = new Date() + } + if (hash) { + updateObject.hash = hash; + } if (target.drafts) { await Promise.all([ - UPDATE.entity(target).where(keys).set({ status, lastScan: new Date() }), + UPDATE.entity(target).where(where).set(updateObject), UPDATE.entity(target.drafts) - .where(keys) - .set({ status, lastScan: new Date() }), + .where(where) + .set(updateObject), ]) } else { await UPDATE.entity(target) - .where(keys) - .set({ status, lastScan: new Date() }) + .where(where) + .set(updateObject) } LOG.info( - `Updated scan status to ${status} for ${target.name}, ${JSON.stringify(keys)}`, + `Updated scan status to ${status} for ${target.name}, ${JSON.stringify(where)}`, ) } diff --git a/tests/incidents-app/package.json b/tests/incidents-app/package.json index 4958f4b3..c098b9bb 100644 --- a/tests/incidents-app/package.json +++ b/tests/incidents-app/package.json @@ -19,11 +19,10 @@ "requires": { "queue": { "parallel": true, - "chunkSize": 20 - }, - "attachments": { - "scanExpiryMs": 30000 + "chunkSize": 50, + "maxAttempts": 2 }, + "auth": { "[development]": { "users": { diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index c1246df7..3e2d2e94 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -3,7 +3,6 @@ const { RequestSend } = require("../utils/api") const { waitForScanStatus, newIncident, - delay, waitForMalwareDeletion, waitForDeletion, runWithUser, @@ -264,9 +263,8 @@ describe("Tests for uploading/deleting attachments through API calls", () => { ) expect(contentResponse.status).toEqual(200) expect(contentResponse.data).toBeTruthy() - - // Wait for 45 seconds to let the scan status expire - await delay(45 * 1000) + const Incidents_attachments = cds.entities('sap.capire.incidents')['Incidents.attachments']; + await UPDATE.entity(Incidents_attachments).where({ID: sampleDocID}).set({lastScan: '2020-01-01T00:00:00'}); await GET( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content`, @@ -1851,7 +1849,6 @@ describe("Tests for uploading/deleting attachments through API calls", () => { ) expect(resultResponse.status).toEqual(200) - const scanCleanWaiter2 = waitForScanStatus("Clean") try { await waitForDeletion(attachmentResponse.data.value[0].url) // Should throw due to timeout @@ -1862,11 +1859,6 @@ describe("Tests for uploading/deleting attachments through API calls", () => { ) } - // Second scan round needed due to scan expiry limit for other tests. Triggered via rescan - await GET( - `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content`, - ) - await scanCleanWaiter2 const contentAfterActiveUpdate = await GET( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content`, ) diff --git a/tests/unit/malwareScanner.test.js b/tests/unit/malwareScanner.test.js index 04828384..afa85ecc 100644 --- a/tests/unit/malwareScanner.test.js +++ b/tests/unit/malwareScanner.test.js @@ -99,6 +99,7 @@ describe("scanAttachmentsFile", () => { expect.anything(), expect.anything(), "Infected", + expect.anything(), ) }) @@ -112,6 +113,7 @@ describe("scanAttachmentsFile", () => { expect.anything(), expect.anything(), "Clean", + expect.anything(), ) }) }) diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 81e71c2f..6efe92ae 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -30,7 +30,9 @@ async function waitForScanStatus(status, attachmentID) { .where.some((e) => e.val && e.val === attachmentID)) || (req.query.UPDATE.where && req.query.UPDATE.where.some( + (e) => (e.val && e.val === attachmentID) || (e.xpr && e.xpr.some( (e) => e.val && e.val === attachmentID, + )), ))) ) { // Store the latest status for timeout reporting From 8de13486a731f37c7e014a5c08b8810b990c77b2 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 15 Apr 2026 14:53:56 +0200 Subject: [PATCH 24/26] Formatting --- srv/malware-scanner/malwareScanner.js | 47 +++++++++++++++++---------- tests/incidents-app/package.json | 1 - tests/integration/attachments.test.js | 8 +++-- tests/utils/testUtils.js | 6 ++-- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/srv/malware-scanner/malwareScanner.js b/srv/malware-scanner/malwareScanner.js index 41a61f1c..2862e4ab 100644 --- a/srv/malware-scanner/malwareScanner.js +++ b/srv/malware-scanner/malwareScanner.js @@ -100,15 +100,30 @@ class MalwareScanner extends cds.ApplicationService { } // Assign hash as another condition to ensure the correct file is marked as fine - await this.updateStatus(_target, ['(', {ref: ['hash']}, '=', {val: hash}, 'or', {ref: ['hash']}, 'is', 'null', ')', 'and', {xpr: Object.keys(keys).reduce((acc, key) => { - if (acc.length) acc.push('and') - acc.push( - {ref: [key]}, - '=', - {val: keys[key]} - ) - return acc; - }, [])}], status, hash) + await this.updateStatus( + _target, + [ + "(", + { ref: ["hash"] }, + "=", + { val: hash }, + "or", + { ref: ["hash"] }, + "is", + "null", + ")", + "and", + { + xpr: Object.keys(keys).reduce((acc, key) => { + if (acc.length) acc.push("and") + acc.push({ ref: [key] }, "=", { val: keys[key] }) + return acc + }, []), + }, + ], + status, + hash, + ) } async _scanWithRetry(AttachmentsSrv, model, target, keys) { @@ -172,24 +187,20 @@ class MalwareScanner extends cds.ApplicationService { } async updateStatus(target, where, status, hash) { - const updateObject = { status }; - if (status !== 'Scanning') { + const updateObject = { status } + if (status !== "Scanning") { updateObject.lastScan = new Date() } if (hash) { - updateObject.hash = hash; + updateObject.hash = hash } if (target.drafts) { await Promise.all([ UPDATE.entity(target).where(where).set(updateObject), - UPDATE.entity(target.drafts) - .where(where) - .set(updateObject), + UPDATE.entity(target.drafts).where(where).set(updateObject), ]) } else { - await UPDATE.entity(target) - .where(where) - .set(updateObject) + await UPDATE.entity(target).where(where).set(updateObject) } LOG.info( `Updated scan status to ${status} for ${target.name}, ${JSON.stringify(where)}`, diff --git a/tests/incidents-app/package.json b/tests/incidents-app/package.json index c098b9bb..c010759f 100644 --- a/tests/incidents-app/package.json +++ b/tests/incidents-app/package.json @@ -22,7 +22,6 @@ "chunkSize": 50, "maxAttempts": 2 }, - "auth": { "[development]": { "users": { diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index 3e2d2e94..bf7905ae 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -263,8 +263,12 @@ describe("Tests for uploading/deleting attachments through API calls", () => { ) expect(contentResponse.status).toEqual(200) expect(contentResponse.data).toBeTruthy() - const Incidents_attachments = cds.entities('sap.capire.incidents')['Incidents.attachments']; - await UPDATE.entity(Incidents_attachments).where({ID: sampleDocID}).set({lastScan: '2020-01-01T00:00:00'}); + const Incidents_attachments = cds.entities("sap.capire.incidents")[ + "Incidents.attachments" + ] + await UPDATE.entity(Incidents_attachments) + .where({ ID: sampleDocID }) + .set({ lastScan: "2020-01-01T00:00:00" }) await GET( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content`, diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 6efe92ae..01f477f0 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -30,9 +30,9 @@ async function waitForScanStatus(status, attachmentID) { .where.some((e) => e.val && e.val === attachmentID)) || (req.query.UPDATE.where && req.query.UPDATE.where.some( - (e) => (e.val && e.val === attachmentID) || (e.xpr && e.xpr.some( - (e) => e.val && e.val === attachmentID, - )), + (e) => + (e.val && e.val === attachmentID) || + (e.xpr && e.xpr.some((e) => e.val && e.val === attachmentID)), ))) ) { // Store the latest status for timeout reporting From 9e512db913c66d7b9e2dd04a98fa5ca4850cc937 Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 15 Apr 2026 15:29:14 +0200 Subject: [PATCH 25/26] Update testUtils.js --- tests/utils/testUtils.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 01f477f0..1cfd858d 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -46,9 +46,10 @@ async function waitForScanStatus(status, attachmentID) { } db.after("*", handler) }), - delay(40000).then(() => { + delay(40000).then(async () => { + const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') throw new Error( - `Timeout waiting for attachment ${attachmentID || ""} to reach status: ${status}, last known status: ${latestStatus}`, + `Timeout waiting for attachment ${attachmentID || ""} to reach status: ${status}, last known status: ${latestStatus}. ${messagesAmount} in outbox.`, ) }), ]) @@ -74,9 +75,10 @@ async function waitForDeletion(attachmentID) { } AttachmentsSrv.on("DeleteAttachment", handler) }), - delay(30000).then(() => { + delay(30000).then(async () => { + const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') throw new Error( - `Timeout waiting for deletion of attachment ID: ${attachmentID}`, + `Timeout waiting for deletion of attachment ID: ${attachmentID}. ${messagesAmount} in outbox.`, ) }), ]) @@ -105,9 +107,10 @@ async function waitForMalwareDeletion(attachmentID) { } AttachmentsSrv.on("DeleteInfectedAttachment", handler) }), - delay(30000).then(() => { + delay(30000).then(async () => { + const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') throw new Error( - `Timeout waiting for malware deletion of attachment ID: ${attachmentID}`, + `Timeout waiting for malware deletion of attachment ID: ${attachmentID}. ${messagesAmount} in outbox.`, ) }), ]) From d2b312b2b5364cb4c5892dbe27ae0c9cefb9f5ed Mon Sep 17 00:00:00 2001 From: Marten Schiwek Date: Wed, 15 Apr 2026 15:29:47 +0200 Subject: [PATCH 26/26] Update testUtils.js --- tests/utils/testUtils.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/utils/testUtils.js b/tests/utils/testUtils.js index 1cfd858d..2022f241 100644 --- a/tests/utils/testUtils.js +++ b/tests/utils/testUtils.js @@ -47,9 +47,11 @@ async function waitForScanStatus(status, attachmentID) { db.after("*", handler) }), delay(40000).then(async () => { - const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') + const { messagesAmount } = await SELECT.one + .from("cds.outbox.Messages") + .columns("count(1) as messagesAmount") throw new Error( - `Timeout waiting for attachment ${attachmentID || ""} to reach status: ${status}, last known status: ${latestStatus}. ${messagesAmount} in outbox.`, + `Timeout waiting for attachment ${attachmentID || ""} to reach status: ${status}, last known status: ${latestStatus}. ${messagesAmount} messages in outbox.`, ) }), ]) @@ -76,9 +78,11 @@ async function waitForDeletion(attachmentID) { AttachmentsSrv.on("DeleteAttachment", handler) }), delay(30000).then(async () => { - const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') + const { messagesAmount } = await SELECT.one + .from("cds.outbox.Messages") + .columns("count(1) as messagesAmount") throw new Error( - `Timeout waiting for deletion of attachment ID: ${attachmentID}. ${messagesAmount} in outbox.`, + `Timeout waiting for deletion of attachment ID: ${attachmentID}. ${messagesAmount} messages in outbox.`, ) }), ]) @@ -108,9 +112,11 @@ async function waitForMalwareDeletion(attachmentID) { AttachmentsSrv.on("DeleteInfectedAttachment", handler) }), delay(30000).then(async () => { - const {messagesAmount} = await SELECT.one.from('cds.outbox.Messages').columns('count(1) as messagesAmount') + const { messagesAmount } = await SELECT.one + .from("cds.outbox.Messages") + .columns("count(1) as messagesAmount") throw new Error( - `Timeout waiting for malware deletion of attachment ID: ${attachmentID}. ${messagesAmount} in outbox.`, + `Timeout waiting for malware deletion of attachment ID: ${attachmentID}. ${messagesAmount} messages in outbox.`, ) }), ])